Что не так в этой функции?

Question

Я думаю, что есть проблема с отношением malloc и goto.Или, я думаю, что здесь происходит потеря памяти или повреждение памяти.Надеюсь, кто-то может указать мне точную ошибку.Когда я компилирую, это не дает мне никакой ошибки, но мой старший настаивает на том, что у меня есть ошибка.

#define FINISH() goto fini;

BOOL Do()
{

    BOOL stat;
    UINT32 ptr;
    int err;

    ptr = (UINT32)malloc(1000);


    free((void*)ptr);

fini:
    return stat;
}

Answer 1

Вот проблемы, которые я заметил в коде

• Когда err != ERROR_SUCCESS эта функция будет утечка памяти. Он перепрыгнет через вызов free.
• Вы сохраняете возвращение malloc в 32-битное местоположение. Это не портативное решение. На 64-битных платформах это приведет к хаосу в вашей программе, так как вы будете сокращать адрес. Если вам необходимо использовать тип без указателя, используйте вместо него size_t (хотя я бы рекомендовал указатель на целочисленный тип)
• Местный stat не назначен здесь окончательно. Вы возвращаете мусор, если err != ERROR_SUCCESS. Ему всегда нужно присваивать значение. Самый простой способ - указать значение по умолчанию.
• Вы не проверяете возвращаемое значение malloc и потенциально можете передать скрытый указатель NULL в Fun2

Вот функция с предложенными мною правками

BOOL Do()
{

    BOOL stat = FALSE;
    size_t ptr = 0;
    int err;

    ptr = (UINT32)malloc(1000);
    err = Fun1();

    if (err != ERROR_SUCCESS || ptr == 0)
        FINISH();
    else
        stat = Fun2(ptr);

fini:
    free((void*)ptr);
    return stat;
}

Answer 2

malloc возвращает указатель.Вы приводите указатель на целое число, но указатели и целые числа не обязательно должны иметь одинаковое представление.Например, размер указателя может быть 64-разрядным и не вписывается в ваше целое число.

Также объект stat может использоваться неинициализированным в вашей функции.Без явной инициализации объект stat имеет неопределенное значение после его объявления.

Answer 3

Вы переводите указатель в uint32_t и обратно.Это стирает верхнюю половину вашего значения указателя.

Answer 4

Мы не знаем, что это должно делать, но если Fun1() не возвращает ERROR_SUCCESS, то ptr никогда не освобождается.Предположительно, об этой ошибке говорит ваш босс.

Answer 5

Мой общий комментарий и общее практическое правило для работы в C ... Если вам нужно выполнить приведение указателей, спросите себя: действительно ли вы должны это делать?Времена, когда вам действительно нужно делать приведения указателей, чрезвычайно редки.Гораздо чаще встречаются случаи, когда люди используют приведение указателей, потому что у них есть некоторый пробел в понимании, они не очень четко понимают, что они пытаются или должны делать, и пытаются заставить замолчать предупреждение компилятора.

ptr = (UINT32)malloc(1000);

Очень плохо!Если вы сделаете что-нибудь с этим «указателем», вам очень повезет, если он работает на 64-битных платформах.Оставьте указатели как типы указателей.Если вам абсолютно необходимо хранить их в целом числе, используйте uintptr_t, который гарантированно будет достаточно большим.

Я бы сказал, что вы, возможно, пытались сделать:

// Allocate 1,000 32-bit integers
UINT32 *ptr = (UINT32*)malloc(1000 * sizeof(UINT32));

Однакодаже это плохая форма для кода C, странный гибрид C и C ++.В отличие от C ++, в C вы можете просто взять void * и неявно привести его к любому типу указателя:

// Allocate 1,000 32-bit integers
UINT32 *ptr = malloc(1000 * sizeof(UINT32));

Наконец,

free((void*)ptr);

Приведение к void* - это еще один большойкрасный флаг, часто признак того, что автор не знает, что он делает.Как только вы измените ptr на фактический тип указателя, просто сделайте это:

free(ptr);

Answer 6

Что бы вы ни делали, вы не компилируете этот код. У него есть синтаксическая ошибка.

if(foo)
  bar;;
else
  baz

Проверьте вашу систему сборки.