Каков наилучший способ освободить память после возврата из ошибки? - PullRequest
Новая
       15

Каков наилучший способ освободить память после возврата из ошибки?

11 голосов
/ 20 февраля 2009

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

int func(void **mem1, void **mem2) {
    *mem1 = malloc(SIZE);
    if (!*mem1) return 1;

    *mem2 = malloc(SIZE);
    if (!*mem2) {
        /* ... */
        return 1;
    }

    return 0;
}

Я хотел бы услышать ваши отзывы о том, как лучше всего освободить () выделенную память в случае сбоя второго malloc (). Вы можете представить себе более сложную ситуацию с большим количеством точек выхода ошибки и большим количеством выделенной памяти.

Ответы [ 9 ]

25 голосов
/ 20 февраля 2009

Я знаю, что люди не любят их использовать, но это идеальная ситуация для goto в С. 

int func( void** mem1, void** mem2 )
{
    int retval = 0;
    *mem1 = malloc(SIZE);
    if (!*mem1) {
        retval = 1;
        goto err;
    }

    *mem2 = malloc(SIZE);
    if (!*mem2) {
        retval = 1;
        goto err;
    }
// ...     
    goto out;
// ...
err:
    if( *mem1 ) free( *mem1 );
    if( *mem2 ) free( *mem2 );
out:
    return retval;
}
7 голосов
/ 20 февраля 2009

Это где goto подходит, по моему мнению. Раньше я следовал догме против Гото, но изменил ее, когда мне указали, что делать {...} while (0); компилируется в тот же код, но не так легко читается. Просто следуйте некоторым основным правилам, таким как не идти назад с ними, сводить их к минимуму, использовать их только для ошибок и т. Д. * 

int func(void **mem1, void **mem2)
{
    *mem1 = NULL;
    *mem2 = NULL;

    *mem1 = malloc(SIZE);
    if(!*mem1)
        goto err;

    *mem2 = malloc(SIZE);
    if(!*mem2)
        goto err;

    return 0;
err:
    if(*mem1)
        free(*mem1);
    if(*mem2)
        free(*mem2);

    *mem1 = *mem2 = NULL;

    return 1;
}
4 голосов
/ 20 февраля 2009

Это немного спорный, но я думаю goto Подход, используемый в Linux ядро ​​на самом деле работает очень хорошо в этой ситуации: 

int get_item(item_t* item)
{
  void *mem1, *mem2;
  int ret=-ENOMEM;
  /* allocate memory */
  mem1=malloc(...);
  if(mem1==NULL) goto mem1_failed;

  mem2=malloc(...);
  if(mem2==NULL) goto mem2_failed;

  /* take a lock */
  if(!mutex_lock_interruptible(...)) { /* failed */
    ret=-EINTR;
    goto lock_failed;
  }

  /* now, do the useful work */
  do_stuff_to_acquire_item(item);
  ret=0;

  /* cleanup */
  mutex_unlock(...);

lock_failed:
  free(mem2);

mem2_failed:
  free(mem1);

mem1_failed:
  return ret;
}
2 голосов
/ 17 мая 2009

Лично; У меня есть библиотека отслеживания ресурсов (в основном сбалансированное двоичное дерево), и у меня есть оболочки для всех функций выделения.

Ресурсы (такие как память, сокеты, дескрипторы файлов, семафоры и т. Д. - все, что вы выделяете и освобождаете) могут принадлежать к набору.

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

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

Итак, несколько malloc выглядят так: 

mem[0] = malloc_wrapper( error_set, resource_set, 100 );
mem[1] = malloc_wrapper( error_set, resource_set, 50 );
mem[2] = malloc_wrapper( error_set, resource_set, 20 );

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

Итак, когда мне приходит время освободить ресурсы (скажем, в конце функции, где все ресурсы, используемые этой функцией внутри, были помещены в набор), я освобождаю набор. Это всего лишь один вызов функции. 

res_delete_set( resource_set );

Мне не нужно специально проверять ошибки - в моем коде нет if (), проверяющих возвращаемые значения, что делает его поддерживаемым; Я считаю, что профилирование встроенной проверки ошибок нарушает читабельность и, следовательно, удобство обслуживания. У меня просто хороший список вызовов функций.

Это art , мужчина: -)

2 голосов
/ 17 мая 2009

Это удобочитаемая альтернатива: 

int func(void **mem1, void **mem2) {
  *mem1 = malloc(SIZE);
  *mem2 = malloc(SIZE);
  if (!*mem1 || !*mem2) {
    free(*mem2);
    free(*mem1);
    return 1;
  }
  return 0;
}
1 голос
/ 20 февраля 2009

Делает ли вызывающий абонент что-нибудь полезное с блоками памяти, которые были правильно распределены до сбоя? Если нет, вызываемый должен обработать освобождение.

Одной из возможностей эффективной очистки является использование do..while(0), что позволяет break там, где ваш пример return s: 

int func(void **mem1, void **mem2)
{
    *mem1 = NULL;
    *mem2 = NULL;

    do
    {
        *mem1 = malloc(SIZE);
        if(!*mem1) break;

        *mem2 = malloc(SIZE);
        if(!*mem2) break;

        return 0;
    } while(0);

    // free is NULL-safe
    free(*mem1);
    free(*mem2);

    return 1;
}

Если вы делаете много выделений, вы можете использовать вашу freeAll() функцию, чтобы также выполнить очистку здесь.

0 голосов
/ 17 мая 2009

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

int func(void **mem1, void **mem2)
{
    *mem1 = malloc(SIZE);
    if (!*mem1) return 1;

    *mem2 = malloc(SIZE);
    if (!*mem2) {
        /* Insert free statement here */
        free(*mem1);
        return 1;
    }

    return 0;
}

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

0 голосов
/ 17 мая 2009

Я немного испуган всеми рекомендациями для заявления goto!

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

Будучи человеком, который любит реорганизовывать вещи, чтобы свести к минимуму мою возможность забыть вещи (например, очистить указатель), я бы сначала добавил несколько функций. Я предполагаю, что вполне вероятно, что я буду использовать их совсем немного в той же программе. Функция imalloc () будет выполнять операцию malloc с косвенным указателем; ifree () отменит это. cifree () освободит память условно.

Учитывая это, моя версия кода (с третьим аргументом для демонстрации) будет выглядеть так: 

// indirect pointer malloc
int imalloc(void **mem, size_t size)
{
   return (*mem = malloc(size));
}

// indirect pointer free
void ifree(void **mem)
{
   if(*mem)
   {
     free(*mem);
     *mem = NULL;
   }
}

// conditional indirect pointer free
void cifree(int cond, void **mem)
{
  if(!cond)
  {
    ifree(mem);
  }
}

int func(void **mem1, void **mem2, void **mem3)
{
   int result = FALSE;
   *mem1 = NULL;
   *mem2 = NULL;
   *mem3 = NULL;

   if(imalloc(mem1, SIZE))
   {
     if(imalloc(mem2, SIZE))
     {
       if(imalloc(mem3, SIZE))
       {
         result = TRUE;
       }            
       cifree(result, mem2);
     }
     cifree(result, mem1);
   }
  return result;
}

Я предпочитаю иметь только один возврат из функции, в конце. Прыжки между ними быстрые (и, на мой взгляд, довольно грязные). Но, что более важно, позволяет вам непреднамеренно обойти связанный код очистки.

0 голосов
/ 20 февраля 2009

Я склонен создавать функцию переменного аргумента, которая освобождает все не-NULL-указатели. Тогда вызывающая сторона может обработать ошибку: 

void *mem1 = NULL;
void *mem2 = NULL;

if (func(&mem1, &mem2)) {
    freeAll(2, mem1, mem2);
    return 1;
}
...