Как избежать длинной цепочки свободных (или удаляемых) после каждой проверки ошибок в C? - PullRequest
16 голосов
/ 27 июля 2010

Предположим, я пишу свой код очень осторожно и всегда проверяю типы возвращаемых значений для всех функций, которые я вызываю.

Итак, я иду как:

char* function() {
    char* mem = get_memory(100);  // first allocation
    if (!mem) return NULL;

    struct binder* b = get_binder('regular binder');  // second allocation
    if (!b) {
        free(mem);
        return NULL;
    }

    struct file* f = mk_file();  // third allocation
    if (!f) {
        free(mem);
        free_binder(b);
        return NULL;
    }

    // ...
}

Обратите внимание, как быстро free() вещи выходят из-под контроля. Если какая-то функция не работает, я должен освободить каждое выделение раньше. Код быстро становится уродливым, и все, что я делаю, это копирую, вставляю все. Я становлюсь программистом копирования / вставки, еще хуже, если кто-то добавляет оператор где-то посередине, он должен изменить весь код ниже, чтобы вызвать free() для его добавления.

Как опытные программисты на Си решают эту проблему? Я ничего не могу понять.

Спасибо, Бода Чидо.

Ответы [ 6 ]

35 голосов
/ 27 июля 2010

Вы можете определить новую метку , которая освободит ресурсы, и затем вы можете ПОЛУЧИТЬ ее при каждом сбое кода.

char* function()
{
    char* retval = NULL;
    char* mem = get_memory(100);  // first allocation
    if (!mem)
        goto out_error;

    struct binder* b = get_binder('regular binder');  // second allocation
    if (!b)
        goto out_error_mem;

    struct file* f = mk_file();  // third allocation
    if (!f)
        goto out_error_b;

    /* ... Normal code path ... */
    retval = good_value;

  out_error_b:
    free_binder(b);
  out_error_mem:
    free(mem);
  out_error:
    return retval;
}

Управление ошибками с помощью GOTO уже обсуждалось здесь: Допустимое использование goto для управления ошибками в C?

5 голосов
/ 27 июля 2010

Хотя я восхищаюсь вашим подходом к защитному кодированию, и это хорошо. И каждый программист C должен иметь такой менталитет, он может применяться и к другим языкам ...

Я должен сказать, что это одна из полезных вещей в GOTO, несмотря на то, что пуристы скажут иначе, это было бы эквивалентом блока finally, но есть одна особая ошибка, которую я могу увидеть там ...

karlphillip код почти завершен, но .... предположим, что функция была выполнена следующим образом

    char* function() {
      struct file* f = mk_file();  // third allocation
      if (!f) goto release_resources;
      // DO WHATEVER YOU HAVE TO DO.... 
      return some_ptr;
   release_resources:
      free(mem);
      free_binder(b);
      return NULL;
    }

Будь осторожен !!! Это будет зависеть от дизайна и назначения функции, которую вы считаете нужной, которая отложит в сторону ... если бы вы вернулись из функции, подобной этой, вы могли бы в конечном итоге провалиться через метку release_resources .... которая могла бы При возникновении незначительной ошибки все ссылки на указатели в куче исчезают и могут привести к возврату мусора ... поэтому убедитесь, что если у вас есть выделенная память и верните ее обратно, используйте ключевое слово return непосредственно перед меткой, в противном случае память может исчезнуть ... или создать утечку памяти ....

5 голосов
/ 27 июля 2010

Я знаю, что меня за это линчуют, но у меня был друг, который сказал, что они использовали goto для этого.

Затем он сказал мне, что в большинстве случаев этого было недостаточно, и теперь он использовал setjmp() / longjmp(). По сути, он заново изобрел исключения в C ++, но с гораздо меньшей элегантностью.

Тем не менее, поскольку goto может работать, вы можете преобразовать его в нечто, что не использует goto, но отступ быстро выйдет из-под контроля:

char* function() {
    char* result = NULL;
    char* mem = get_memory(100);
    if(mem) {
        struct binder* b = get_binder('regular binder');
        if(b) {
            struct file* f = mk_file();
            if (f) {
                // ...
            }
            free(b);
        }
        free(mem);
    }
    return result;
}

Кстати, рассеяние объявлений локальной переменной вокруг блока, как это не является стандартным C.

Теперь, если вы понимаете, что free(NULL); определяется стандартом C как ничего не делающего, вы можете упростить вложение некоторых:

char* function() {
    char* result = NULL;

    char* mem = get_memory(100);
    struct binder* b = get_binder('regular binder');
    struct file* f = mk_file();

    if (mem && b && f) {
        // ...
    }

    free(f);
    free(b);
    free(mem);

    return result;
}
3 голосов
/ 27 июля 2010

Вы также можете воспользоваться противоположным подходом и проверить успех:

struct binder* b = get_binder('regular binder');  // second allocation
if(b) {
   struct ... *c = ...
   if(c) {
      ...
   }
   free(b);
}
2 голосов
/ 27 июля 2010

Если ваши структуры данных являются сложными / вложенными, одного перехода может быть недостаточно, и в этом случае я предлагаю что-то вроде:

mystruct = malloc(sizeof *mystruct);
if (!mystruct) goto fail1;
mystruct->data = malloc(100);
if (!mystruct->data) goto fail2;
foo = malloc(sizeof *foo);
if (!foo) goto fail2;
...
return mystruct;
fail2:
free(mystruct->data);
fail1:
free(mystruct);

Пример из реального мира будет более сложным и может включать несколько уровнейВложение структуры, связанные списки и т. д. Обратите внимание, что free(mystruct->data); не может быть вызвано (потому что разыменование элемента mystruct недопустимо), если первый malloc не удался.

2 голосов
/ 27 июля 2010

Если вы хотите сделать это без goto, вот подход, который хорошо масштабируется:

char *function(char *param)
{
    int status = 0;   // valid is 0, invalid is 1
    char *result = NULL;
    char *mem = NULL:
    struct binder* b = NULL;
    struct file* f = NULL:

    // check function parameter(s) for validity
    if (param == NULL)
    {
        status = 1;
    }

    if (status == 0)
    {
        mem = get_memory(100);  // first allocation

        if (!mem)
        {
            status = 1;
        }
    }

    if (status == 0)
    {
        b = get_binder('regular binder');  // second allocation

        if (!b)
        {
             status = 1;
        }
    }

    if (status == 0)
    {
        f = mk_file();  // third allocation

        if (!f)
        {
             status = 1;
        }
    }

    if (status == 0)
    {
        // do some useful work
        // assign value to result
    }

    // cleanup in reverse order
    free(f);
    free(b);
    free(mem);

    return result;
}
Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...