LINUX.ORG.RU

Освобождение ресурсов при ошибке

 


4

5
int func(int x)
{
  char * c1 = malloc(SOMESIZE);
  char * c2 = malloc(EVENMORE);

  if ((!c1) || (!c2))
  {
    if (c1) free(c1);
    if (c2) free(c2);
    return E_CANTMALLOC;
  }

  int boolerr = 0;
  int retcode = 0;

  switch (x)
  {
  case 0:
     ...
     if (somebadnews)
     {
        boolerr = 1;
        retcode = E_CODE_GREEN;
     }
     break;
  case 1:
     ...
     if (somebadnews)
     {
        boolerr = 1;
        retcode = E_CODE_YELLOW;
     }
     break;
  case 2:
     ...
     if (somebadnews)
     {
        boolerr = 1;
        retcode = E_CODE_RED;
     }
     break;
  case 3:
     ...
     if (somebadnews)
     {
        boolerr = 1;
        retcode = E_CODE_BLACK;
     }
     break;
  default:
     boolerr = 1;
     retcode = E_CODE_BROWN;
  }

  if (boolerr)
  {
    free(c1);
    free(c2);
    return retcode;
  }

  ...

  return 0;
}

А как по канону нужно обрабатывать ситуацию, когда одна из подпрограмм вернула ошибку и нужно освободить ресурсы и передать наверх тот же код?

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

Eddy_Em ☆☆☆☆☆ ()

Тебе щас напишут почему готу говно и зашквар и будет срач. Пока не начался, скажу — пишешь макрос #define ERR(code) do { retcode = code; goto cleanup; } while (0), а cleanup: делаешь в конце и освобождаешь все, что надо, если оно еще/уже не NULL, и потом return retcode. Нормальный выход тоже через cleanup, разница только в retcode.

arturpub ★★ ()
int func(int x)
{
    int retcode = 0;
    char* c1 = NULL;
    char* c2 = NULL;

    c1 = malloc(SOMESIZE);
    c2 = malloc(EVENMORE);

    if ((!c1) || (!c2))
    {
        retcode = E_CANTMALLOC;
        goto cleanup;
    }

    switch (x)
    {
    case 0:
       ...
       if (somebadnews)
       {
          retcode = E_CODE_GREEN;
          goto cleanup;
       }
       break;
    case 1:
       ...
       if (somebadnews)
       {
          retcode = E_CODE_YELLOW;
          goto cleanup;
       }
       break;
    case 2:
       ...
       if (somebadnews)
       {
          retcode = E_CODE_RED;
          goto cleanup;
       }
       break;
    case 3:
       ...
       if (somebadnews)
       {
          retcode = E_CODE_BLACK;
          goto cleanup;
       }
       break;
    default:
       retcode = E_CODE_BROWN;
    }

    ...

    return 0;

cleanup:
    free(c1);
    free(c2);
    return ret;
}
ilammy ★★★ ()
Ответ на: комментарий от sambist

Нет, это — самый приличный и удобоваримый способ. УМВР. Везде так делаю.

Eddy_Em ☆☆☆☆☆ ()

Бтв, никогда не проверяю возвращаемое значение malloc()/realloc(). Максимум — запрашиваемый размер на адекватность, если он извне приходит.

ilammy ★★★ ()

Канонiчно это было бы так:

enum retval {
        E_CODE_OK,
        E_CODE_GREEN,
        E_CODE_YELLOW,
        E_CODE_RED,
        E_CODE_BLACK,
        E_CODE_BROWN,
        E_CANTMALLOC,
};      

int 
func(int x)
{ 
        enum retval retcode = E_CODE_OK;

        char *c1 = malloc(SOMESIZE);
        char *c2 = malloc(EVENMORE);

        if (!c1 || !c2) {
                retcode = E_CANTMALLOC;
                goto error;
        }
  
        switch (x) {
        case 0:
                /* ... */
                if (somebadnews) {
                        retcode = E_CODE_GREEN;
                        goto error;
                }
                break;
        case 1:
                /* ... */
                if (somebadnews) {
                        retcode = E_CODE_YELLOW;
                        goto error;
                } 
                break;
        case 2:
                /* ... */
                if (somebadnews) {
                        retcode = E_CODE_RED;
                        goto error;
                } 
                break;
        case 3:
                /* ... */
                if (somebadnews) {
                        retcode = E_CODE_BLACK;
                        goto error;
                }
                break;
        default:
                retcode = E_CODE_BROWN;
                goto error;
        }
    
        /* ... */

error:
        if (c1)
                free(c1);
        if (c2) 
                free(c2);
    
        return retcode;
}
beastie ★★★★★ ()
Ответ на: комментарий от arturpub

Можно еще сделать макрос CHK(cond, code), который будет вызывать ERR при !cond, так еще короче. Не забывай undef'ить макросы, если ты в хидере или длинном сорце (чисто эстетика, кому как; если он глобальный, то ок, а если «scoped», то #undef в конце скопа.

arturpub ★★ ()
Ответ на: комментарий от Carb_blog3

Какая хер разница, ты и так проверяешь fopen, socket, write, но маллок это же пипец тяжело, особенно когда у тебя уже есть универсальный макрос. Или ты не проверяешь, у правильных тактое**в все файлы всегда открываются, а коннекты никогда не рвутся?

arturpub ★★ ()

Горжусь лором.

d ★★★★ ()
Ответ на: комментарий от arturpub

Ну, это уже мелочи, главное принцип.

#define GOODNEWS(cond, state) do {       \
                if (cond) {              \
                        retcode = state; \
                        goto error;      \
                }                        \
        } while(0)
beastie ★★★★★ ()
Ответ на: комментарий от beastie

Щас те царь-то задаст... man 3 free, срочно!

Правда это частный случай и хорошая привычка — относительно всех ресурсов делать if (x) { xfree(x); x = NULL; }.

arturpub ★★ ()
Последнее исправление: arturpub (всего исправлений: 1)
Ответ на: комментарий от Carb_blog3

Ты не тот ник себе выбрал. Нужно было malloc-free-po-tsarsky назваться.

nanoolinux ★★★★ ()
Ответ на: комментарий от arturpub

Лучше конечно макрос FREE(x, xfree) написать, чтобы не велосипедить по четыре строки.

arturpub ★★ ()
Ответ на: комментарий от arturpub

Царь, без царя в голове. =)

Смотря какой free. Как минимум на *BSD и Сорлярке упадёт с хрустом. =)

делать if (x) { xfree(x); x = NULL; }.

Да-да, а потом Heartbleed выходит, когда пытаются обойти ванильные функции управления памятью.

beastie ★★★★★ ()
Последнее исправление: beastie (всего исправлений: 1)
Ответ на: комментарий от arturpub

А ничего, что free(NULL) ничего не будет делать?

Ну, а лично у меня есть макрос:

#define FREE(x) do{free(x); x = NULL;}while(0)
и ты уверен, что это FREE можно делать бесконечно!

Eddy_Em ☆☆☆☆☆ ()
Ответ на: комментарий от beastie

Хартблид не из-за этого был, не надо тут :)

Под xfree я имел ввиду нормальный для этого типа x деструктор, fclose там, или gtk_widget_destroy, а не самописный xfree какой-то.

arturpub ★★ ()
Ответ на: комментарий от arturpub

Какая хер разница, ты и так проверяешь fopen, socket, write, но маллок это же пипец тяжело, особенно когда у тебя уже есть универсальный макрос.

Об этом я говорил в теме про кути и ексепшны, только тут у пацана явный:

char *c2 = malloc(EVENMORE);
if (c1)
                free(c1);

И дело далеко не в общем случае.

fopen, socket, write

Убожество «fopen» тут явно лишнее.

Ну давай. Ошибка -1, это валидный вход для close(). Это суть - ошибка из прямой является валидным для обратной, которая вернёт туже ошибку. Это правильно.

Потеряется только errno, но оно в ~любом случае потеряется при данном подходе.

Или ты не проверяешь, у правильных тактое**в все файлы всегда открываются, а коннекты никогда не рвутся?

Это проблема подхода, когда ты будешь лепить не из говна, а с какой-то логикой стейта и отката стейта, то тогда таких проблем не возникнет.

Как будет не лень - я накидаю вам примеров, как это делается у правильных пацанов.

Да и man следилки за фдешками. Она тебе сообщит обо всём. Это пример глобального стейта. Это пример логики. А лапша - это бездарная лапша.

Carb_blog3 ()
Ответ на: комментарий от arturpub

Хотя, беру свои слова обратно. Заглянул в сырцы. На Open тоже можно free(NULL) делать:

void
free(void *ptr)
{
        int saved_errno = errno;

        /* This is legal. */
        if (ptr == NULL)
                return;

        /* ... */
}
А вот по поводу других систем всё ещё не уверен.

beastie ★★★★★ ()
Ответ на: комментарий от beastie

Смотря какой free. Как минимум на *BSD и Сорлярке упадёт с хрустом. =)

Который написан по стандарту. А если я проверю? Какая там либц? Название скажи мне.

The free function causes the space pointed to by ptr to be deallocated, that is, made
available for further allocation. If ptr is a null pointer, no action occurs. Otherwise, if
the argument does not match a pointer earlier returned by a memory management
function, or if the space has been deallocated by a call to free or realloc, the
behavior is undefined.

Да-да, а потом Heartbleed выходит, когда пытаются обойти ванильные функции управления памятью.

Он выходит, когда пацаны, которых к пхп подпускать с «хруском» еле-еле можно, пытаются что-то писать на си.

Carb_blog3 ()
Ответ на: комментарий от arturpub

Не надо этими говнофункциями из stdio пользоваться. Не нужны они. Есть же open, read, write и close. И всякие кривые библиотеки их не пытаются буферизовать!

Eddy_Em ☆☆☆☆☆ ()

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

Смысл в том, что функция делится на две: одна выделает и освобождает ресурсы, а вторая делает всю важную работу. Например, так:

static int func_core(int x, char c1[], char c2[]);

int func(int x)
{
    char *const c1 = malloc(SOMESIZE);
    char *const c2 = malloc(EVENMORE);

    int retcode;

    if (c1 == NULL || c2 == NULL)
    {
        retcode = E_CANTMALLOC;
    }
    else
    {
        retcode = func_core(x, c1, c2);
    }

    free(c1);
    free(c2);

    return retcode;
}

static int func_core(int x, char c1[], char c2[])
{
    switch (x)
    {
    case 0:
        ...
        if (somebadnews)
        {
            return E_CODE_GREEN;
        }
        break;
    case 1:
        ...
        if (somebadnews)
        {
            return E_CODE_YELLOW;
        }
        break;
    case 2:
        ...
        if (somebadnews)
        {
            return E_CODE_RED;
        }
        break;
    case 3:
        ...
        if (somebadnews)
        {
            return E_CODE_BLACK;
        }
        break;

    default:
       return E_CODE_BROWN;
    }

    ...

    return 0;
}

xaizek ★★★★★ ()

Или goto или инвертированная лестница if'ов

anonymous ()

Всё очень просто и никаких goto:

typedef struct {char *c1; char * c2; } Resources;

...
{
 struct Resources r;
 if(Resources_Init(&r))
 {
  Resources_Process(&r);
 }
 Resources_Free(&r);
}

Из Resources_Process можно выходить по return в любой момент.

O02eg ★★★★★ ()
Ответ на: комментарий от Eddy_Em

Мне норм (хотя честно не помню когда взаправду юзал).

arturpub ★★ ()
Ответ на: комментарий от Eddy_Em

Почему?

Я таки, по старой памяти был не прав. Теперь можно. (А вот раньше ИМХО нельзя было.)

ref1: https://github.com/illumos/illumos-gate/blob/master/usr/src/lib/libmalloc/com...

ref2: http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/stdlib/malloc.c?rev=1.169

ref3: http://fossies.org/dox/glibc-2.19/memusage_8c_source.html#l00544

PS: какя же гадость этот ваш glibc! Чёрт ногу сломит.

beastie ★★★★★ ()
Последнее исправление: beastie (всего исправлений: 1)
Ответ на: комментарий от O02eg

typedef struct {char *c1; char * c2; } Resources;
struct Resources r;
if(Resources_Init(&r))

Да ты я смотрю прошаренный чувак. Про скобочки и форматирование я уж так и быть, промолчу.

А так да, если не принимать во внимание познания языка на уровне яслей, вменяемый пример общего стейта.

Carb_blog3 ()
Ответ на: комментарий от Eddy_Em

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

Вы, конечно, напишете много разных goto, для каждой такой фиговины?

O02eg ★★★★★ ()
Ответ на: комментарий от Eddy_Em

Он прав. Такая фиговина должна быть - это признак «логики», а когда её нет - твой код говно. Это глобальный контекст/стейт.

которые инициализируются в разных местах функции.

Так делает только говно, они не инициализируют, а реинициализируют.

Изначальный стейт NULL, после чего ты апдейтишь стейт - если ошибка - откатываешь( у тебя везде будет либо нал, либо поинтер), если нет - хреначишь.

Если пишешь в говностайле - это ничего не мешает. Юзаешь маллоки в Resources_Process().

Carb_blog3 ()
Ответ на: комментарий от xaizek

Эти пацаны:

typedef enum {E_CODE_GREEN = 0, E_CODE_YELLOW, E_CODE_RED, E_CODE_BLACK, E_CODE_BROWN} error_t;
static error_t func_core(int x) {
  return (x > 3) ? E_CODE_BROWN : (int[]){E_CODE_GREEN, E_CODE_YELLOW, E_CODE_RED, E_CODE_BLACK}[x];
}

Лапша, это же так круто, да? Кто вас учит? Что вы пишите?

Carb_blog3 ()
Ответ на: комментарий от O02eg

Нет, ни одного. goto в макросе. В каждой функции на выходе — метка. Все метки одинаковые.

Eddy_Em ☆☆☆☆☆ ()
Ответ на: комментарий от Carb_blog3

реинициализируют

Инициализируют. В смысле — выделяют память. А изначальное определение переменных вначале должно быть. Где всем пишется var *ptr = NULL;. Тут тоже лучше макросы забульбенить.

Eddy_Em ☆☆☆☆☆ ()
Ответ на: комментарий от Carb_blog3

Что? Это код автора темы, который я переделал с целью продемонстрировать смысл сказанного. Предполагается, что так ему будет проще понять слова, чем если привести левый код.

xaizek ★★★★★ ()
Ответ на: комментарий от Carb_blog3

Не первый год пишем, все попробовано. Мне лично неудобно императивный поток мыслей излагать в виде only readable today стейт-машины; если так извращаться, лучше кресты взять, там хоть само.

зы: офк не лучше, и не само, но смысл думаю ясен.

arturpub ★★ ()
Ответ на: комментарий от O02eg

Как уже сказали — все в макросах, ничего не висит. Имхо людей по большей части пугают страшилками про гоуту и макросы и прочая, а альтернативы никакой не дают, кроме лапши, фсм, или отступов в 40 пробелов, отсюда множество проблем с архитектурой — тупо неудобно писать. Идиотизм. Я бы вообще с радостью на m4 или чем посерьезнее макросы клепал вместо cpp, хоть какой-то лисп^Wпростор, но оно увы не из коробки.

arturpub ★★ ()
Ответ на: комментарий от Carb_blog3

Ну давай. Ошибка -1, это валидный вход для close(). Это суть - ошибка из прямой является валидным для обратной, которая вернёт туже ошибку. Это правильно.

Не уловил. Имеешь ввиду, можно не париться и писать...

int fd = open(...);
write(fd, ...);
close(fd);
... потому что -1 проскочит сквозь этот блок нопом? Или не то?

man следилки за фдешками

poll/select/aio/etc?

arturpub ★★ ()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.