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

Этот вопрос на самом деле является результатом интересного обсуждения на сайте programming.reddit.com некоторое время назад. В основном это сводится к следующему коду:

int foo(int bar)
{
    int return_value = 0;
    if (!do_something( bar )) {
        goto error_1;
    }
    if (!init_stuff( bar )) {
        goto error_2;
    }
    if (!prepare_stuff( bar )) {
        goto error_3;
    }
    return_value = do_the_thing( bar );
error_3:
    cleanup_3();
error_2:
    cleanup_2();
error_1:
    cleanup_1();
    return return_value;
}

Использование gotohere кажется лучшим путем, результатом которого является самый чистый и эффективный код из всех возможных, по крайней мере, мне так кажется. Цитата Стива МакКоннелла в Code Complete :

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

Еще одна поддержка этого подхода содержится в книге « Драйверы устройств Linux» в этом разделе .

Что вы думаете? Подходит ли этот случай для gotoC? Вы бы предпочли другие методы, которые производят более запутанный и / или менее эффективный код, но избегаете goto?

FWIF, я считаю, что идиома обработки ошибок, которую вы указали в примере вопроса, более читабельна и легче для понимания, чем любая из альтернатив, приведенных в ответах на данный момент. Хотя gotoв целом это плохая идея, она может быть полезна для обработки ошибок, если выполняется простым и единообразным образом. В этой ситуации, даже если это goto, он используется четко определенным и более или менее структурированным образом.

Как правило, избегать goto - это хорошая идея, но злоупотребления, которые были распространены, когда Дейкстра впервые написал «GOTO Считается вредным», в наши дни даже не приходят в голову большинству людей.

То, что вы описываете, является обобщаемым решением проблемы обработки ошибок - меня это устраивает, если его тщательно использовать.

Ваш конкретный пример можно упростить следующим образом (шаг 1):

int foo(int bar)
{
    int return_value = 0;
    if (!do_something(bar)) {
        goto error_1;
    }
    if (!init_stuff(bar)) {
        goto error_2;
    }
    if (prepare_stuff(bar))
    {
        return_value = do_the_thing(bar);
        cleanup_3();
    }
error_2:
    cleanup_2();
error_1:
    cleanup_1();
    return return_value;
}

Продолжая процесс:

int foo(int bar)
{
    int return_value = 0;
    if (do_something(bar))
    {   
        if (init_stuff(bar))
        {
            if (prepare_stuff(bar))
            {
                return_value = do_the_thing(bar);
                cleanup_3();
            }
            cleanup_2();
        }
        cleanup_1();
    }
    return return_value;
}

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

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

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

gotoСначала я покажу версию, потому что она ближе к коду в исходном вопросе.

int foo(int bar)
{
    int return_value = 0;
    int something_done = 0;
    int stuff_inited = 0;
    int stuff_prepared = 0;


    /*
     * Prepare
     */
    if (do_something(bar)) {
        something_done = 1;
    } else {
        goto cleanup;
    }

    if (init_stuff(bar)) {
        stuff_inited = 1;
    } else {
        goto cleanup;
    }

    if (prepare_stuff(bar)) {
        stufF_prepared = 1;
    } else {
        goto cleanup;
    }

    /*
     * Do the thing
     */
    return_value = do_the_thing(bar);

    /*
     * Clean up
     */
cleanup:
    if (stuff_prepared) {
        unprepare_stuff();
    }

    if (stuff_inited) {
        uninit_stuff();
    }

    if (something_done) {
        undo_something();
    }

    return return_value;
}

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

Кто-то может возразить, что этот метод добавляет множество дополнительных переменных - и в данном случае это действительно так, - но на практике часто существующая переменная уже отслеживает или может быть настроена для отслеживания требуемого состояния. Например, если это на prepare_stuff()самом деле вызов malloc()или open(), то можно использовать переменную, содержащую возвращаемый указатель или дескриптор файла - например:

int fd = -1;

....

fd = open(...);
if (fd == -1) {
    goto cleanup;
}

...

cleanup:

if (fd != -1) {
    close(fd);
}

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

int foo(int bar)
{
    int return_value = 0;
    int something_done = 0;
    int stuff_inited = 0;
    int stuff_prepared = 0;
    int oksofar = 1;


    /*
     * Prepare
     */
    if (oksofar) {  /* NB This "if" statement is optional (it always executes) but included for consistency */
        if (do_something(bar)) {
            something_done = 1;
        } else {
            oksofar = 0;
        }
    }

    if (oksofar) {
        if (init_stuff(bar)) {
            stuff_inited = 1;
        } else {
            oksofar = 0;
        }
    }

    if (oksofar) {
        if (prepare_stuff(bar)) {
            stuff_prepared = 1;
        } else {
            oksofar = 0;
        }
    }

    /*
     * Do the thing
     */
    if (oksofar) {
        return_value = do_the_thing(bar);
    }

    /*
     * Clean up
     */
    if (stuff_prepared) {
        unprepare_stuff();
    }

    if (stuff_inited) {
        uninit_stuff();
    }

    if (something_done) {
        undo_something();
    }

    return return_value;
}

Опять же, есть потенциальная критика этого:

• Разве все эти «если» не вредит производительности? Нет - потому что в случае успеха вы все равно должны выполнить все проверки (иначе вы не проверяете все случаи ошибок); и в случае сбоя большинство компиляторов оптимизируют последовательность неудачных if (oksofar)проверок до одного перехода к коду очистки (GCC, безусловно, делает) - и в любом случае случай ошибки обычно менее критичен для производительности.

• Разве это не добавление еще одной переменной? В этом случае да, но часто return_valueпеременная может использоваться, чтобы играть роль, которая oksofarздесь играет. Если вы структурируете свои функции так, чтобы они возвращали ошибки последовательным образом, вы даже можете избежать второго ifв каждом случае:

  int return_value = 0;
  
  if (!return_value) {
      return_value = do_something(bar);
  }
  
  if (!return_value) {
      return_value = init_stuff(bar);
  }
  
  if (!return_value) {
      return_value = prepare_stuff(bar);
  }
  

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

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

Проблема с gotoключевым словом в основном понимается неправильно. Это не просто зло. Вам просто нужно знать о дополнительных путях управления, которые вы создаете с каждым goto. Становится сложно рассуждать о вашем коде и, следовательно, о его достоверности.

FWIW, если вы посмотрите руководства на developer.apple.com, они используют подход goto для обработки ошибок.

Мы не используем gotos. Более важное значение придается возвращаемым значениям. Обработка исключений осуществляется с помощью всего, setjmp/longjmpчто вы можете.

В выражении goto нет ничего морально неправильного, как и в указателях (void) *.

Все дело в том, как вы используете инструмент. В (тривиальном) случае, который вы представили, оператор case может достичь той же логики, хотя и с большими накладными расходами. Настоящий вопрос: каковы мои требования к скорости?

goto просто быстр, особенно если вы внимательно следите за тем, чтобы он компилировался в короткий прыжок. Идеально подходит для приложений, где скорость важна. Для других приложений, вероятно, имеет смысл уменьшить накладные расходы с помощью if / else + case для удобства обслуживания.

Помните: goto не убивает приложения, разработчики убивают приложения.

ОБНОВЛЕНИЕ: вот пример случая

int foo(int bar) { 
     int return_value = 0 ; 
     int failure_value = 0 ;

     if (!do_something(bar)) { 
          failure_value = 1; 
      } else if (!init_stuff(bar)) { 
          failure_value = 2; 
      } else if (prepare_stuff(bar)) { 
          return_value = do_the_thing(bar); 
          cleanup_3(); 
      } 

      switch (failure_value) { 
          case 2: cleanup_2(); 
          case 1: cleanup_1(); 
          default: break ; 
      } 
} 

GOTO полезно. Это то, что может делать ваш процессор, и поэтому у вас должен быть к нему доступ.

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

В общем, я бы рассмотрел тот факт, что часть кода может быть наиболее четко написана, используя gotoкак признак того, что поток программы, вероятно, более сложен, чем обычно желательно. Комбинирование других программных структур странными способами во избежание использования gotoбудет пытаться лечить симптом, а не болезнь. Ваш конкретный пример может быть не слишком сложно реализовать без goto:

  делать {
    .. настроить вещь1, которая будет нуждаться в очистке только в случае досрочного выхода
    если (ошибка) перерыв;
    делать
    {
      .. настроить thing2, который будет нуждаться в очистке в случае досрочного выхода
      если (ошибка) перерыв;
      // ***** СМ. ТЕКСТ ОТНОСИТЕЛЬНО ЭТОЙ СТРОКИ
    } while (0);
    .. cleanup thing2;
  } while (0);
  .. cleanup thing1;

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

В сценарии «очистка даже в нормальном случае» я считаю использование gotoболее понятным, чем конструкции do/ while(0), среди прочего потому, что сами целевые метки практически кричат ​​«ПОСМОТРИТЕ НА МЕНЯ» гораздо чаще, чем конструкции breakи do/ while(0). Для случая «очистка только в случае ошибки» returnоператор оказывается примерно в худшем месте с точки зрения удобочитаемости (операторы возврата обычно должны быть либо в начале функции, либо в том, что «выглядит» конец); наличие returnметки непосредственно перед тем, как целевая метка соответствует этой квалификации, гораздо легче, чем наличие метки непосредственно перед концом «цикла».

Кстати, один сценарий, который я иногда использую gotoдля обработки ошибок, находится внутри switchоператора, когда код для нескольких случаев имеет один и тот же код ошибки. Несмотря на то, что мой компилятор часто был достаточно умен, чтобы распознавать, что несколько случаев заканчиваются одним и тем же кодом, я думаю, что более ясно сказать:

 REPARSE_PACKET:
  коммутатор (пакет [0])
  {
    case PKT_THIS_OPERATION:
      если (условие проблемы)
        goto PACKET_ERROR;
      ... обработать THIS_OPERATION
      перемена;
    case PKT_THAT_OPERATION:
      если (условие проблемы)
        goto PACKET_ERROR;
      ... обрабатывать THAT_OPERATION
      перемена;
    ...
    case PKT_PROCESS_CONDITIONALLY
      если (длина_пакета <9)
        goto PACKET_ERROR;
      if (условие_пакета с участием пакета [4])
      {
        длина_пакета - = 5;
        memmove (пакет, пакет + 5, длина_пакета);
        goto REPARSE_PACKET;
      }
      еще
      {
        пакет [0] = PKT_CONDITION_SKIPPED;
        пакет [4] = длина_пакета;
        длина_пакета = 5;
        packet_status = READY_TO_SEND;
      }
      перемена;
    ...
    по умолчанию:
    {
     PACKET_ERROR:
      packet_error_count ++;
      длина_пакета = 4;
      пакет [0] = PKT_ERROR;
      packet_status = READY_TO_SEND;
      перемена;
    }
  }   

Хотя можно заменить gotoоператоры на {handle_error(); break;}, и хотя можно использовать цикл do/ while(0)вместе с continueдля обработки обернутого пакета условного выполнения, я действительно не думаю, что это яснее, чем использование goto. Кроме того, хотя можно было бы скопировать код PACKET_ERRORотовсюду, где goto PACKET_ERRORон используется, и хотя компилятор может выписать дублированный код один раз и заменить большинство вхождений переходом к этой общей копии, использование gotoтега упрощает обнаружение мест которые устанавливают пакет немного по-другому (например, если команда «выполнить условно» решает не выполнять).

Я лично являюсь сторонником «Сила десяти - 10 правил написания критического кода безопасности» .

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

Правило: ограничьте весь код очень простыми конструкциями потока управления - не используйте операторы goto, конструкции setjmp или longjmp, а также прямую или косвенную рекурсию.

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

Запрет использования goto кажется плохим, но:

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

Я согласен с тем, что очистка goto в обратном порядке, указанном в вопросе, является самым чистым способом очистки большинства функций. Но я также хотел отметить, что иногда вы хотите, чтобы ваша функция все равно очищалась. В этих случаях я использую следующий вариант if (0) {label:} идиома, чтобы перейти к нужной точке процесса очистки:

int decode ( char * path_in , char * path_out )
{
  FILE * in , * out ;
  code c ;
  int len ;
  int res = 0  ;
  if ( path_in == NULL )
    in = stdin ;
  else
    {
      if ( ( in = fopen ( path_in , "r" ) ) == NULL )
        goto error_open_file_in ;
    }
  if ( path_out == NULL )
    out = stdout ;
  else
    {
      if ( ( out = fopen ( path_out , "w" ) ) == NULL )
        goto error_open_file_out ;
    }

  if( read_code ( in , & c , & longueur ) )
    goto error_code_construction ;

  if ( decode_h ( in , c , out , longueur ) )
  goto error_decode ;

  if ( 0 ) { error_decode: res = 1 ;}
  free_code ( c ) ;
  if ( 0 ) { error_code_construction: res = 1 ; }
  if ( out != stdout ) fclose ( stdout ) ;
  if ( 0 ) { error_open_file_out: res = 1 ; }
  if ( in != stdin ) fclose ( in ) ;
  if ( 0 ) { error_open_file_in: res = 1 ; }
  return res ;
 }

Мне кажется, что cleanup_3надо сделать очистку, а затем позвонить cleanup_2. Точно так же cleanup_2следует выполнить очистку, а затем вызвать cleanup_1. Похоже, что всякий раз, когда вы это делаете cleanup_[n], это cleanup_[n-1]необходимо, поэтому это должно быть ответственностью метода (так что, например, его cleanup_3нельзя вызвать без вызова cleanup_2и, возможно, вызвать утечку)

При таком подходе вместо gotos вы просто вызываете процедуру очистки, а затем возвращаетесь.

Этот gotoподход не является неправильным или плохим , просто стоит отметить, что это не обязательно «самый чистый» подход (ИМХО).

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

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

Рассмотреть возможность:

1. do_something (), init_stuff () и prepare_stuff (), похоже, знают, потерпели ли они неудачу, поскольку в этом случае они возвращают либо false, либо nil.
2. Ответственность за настройку состояния, по-видимому, лежит на этих функциях, поскольку состояние не настраивается непосредственно в foo ().

Следовательно: do_something (), init_stuff () и prepare_stuff () должны выполнять свою собственную очистку . Наличие отдельной функции cleanup_1 (), которая очищает после do_something (), нарушает философию инкапсуляции. Плохой дизайн.

Если они сделали свою очистку, то foo () станет довольно простым.

С другой стороны. Если foo () действительно создала собственное состояние, которое нужно было удалить, тогда подойдет goto.

Вот что я предпочел:

bool do_something(void **ptr1, void **ptr2)
{
    if (!ptr1 || !ptr2) {
        err("Missing arguments");
        return false;
    }
    bool ret = false;

    //Pointers must be initialized as NULL
    void *some_pointer = NULL, *another_pointer = NULL;

    if (allocate_some_stuff(&some_pointer) != STUFF_OK) {
        err("allocate_some_stuff step1 failed, abort");
        goto out;
    }
    if (allocate_some_stuff(&another_pointer) != STUFF_OK) {
        err("allocate_some_stuff step 2 failed, abort");
        goto out;
    }

    void *some_temporary_malloc = malloc(1000);

    //Do something with the data here
    info("do_something OK");

    ret = true;

    // Assign outputs only on success so we don't end up with
    // dangling pointers
    *ptr1 = some_pointer;
    *ptr2 = another_pointer;
out:
    if (!ret) {
        //We are returning an error, clean up everything
        //deallocate_some_stuff is a NO-OP if pointer is NULL
        deallocate_some_stuff(some_pointer);
        deallocate_some_stuff(another_pointer);
    }
    //this needs to be freed every time
    free(some_temporary_malloc);
    return ret;
}

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

static inline int foo_2(int bar)
{
    int return_value = 0;
    if ( prepare_stuff( bar ) ) {
        return_value = do_the_thing( bar );
    }
    cleanup_3();
    return return_value;
}

static inline int foo_1(int bar)
{
    int return_value = 0;
    if ( init_stuff( bar ) ) {
        return_value = foo_2(bar);
    }
    cleanup_2();
    return return_value;
}

int foo(int bar)
{
    int return_value = 0;
    if (do_something(bar)) {
        return_value = foo_1(bar);
    }
    cleanup_1();
    return return_value;
}

Что касается места, мы создаем в три раза больше переменной в стеке, что нехорошо, но это исчезает при компиляции с -O2, удаляя переменную из стека и используя регистр в этом простом примере. Из приведенного выше блока я получил gcc -S -O2 test.cследующее:

    .section    __TEXT,__text,regular,pure_instructions
    .macosx_version_min 10, 13
    .globl  _foo                    ## -- Begin function foo
    .p2align    4, 0x90
_foo:                                   ## @foo
    .cfi_startproc
## %bb.0:
    pushq   %rbp
    .cfi_def_cfa_offset 16
    .cfi_offset %rbp, -16
    movq    %rsp, %rbp
    .cfi_def_cfa_register %rbp
    pushq   %r14
    pushq   %rbx
    .cfi_offset %rbx, -32
    .cfi_offset %r14, -24
    movl    %edi, %ebx
    xorl    %r14d, %r14d
    xorl    %eax, %eax
    callq   _do_something
    testl   %eax, %eax
    je  LBB0_6
## %bb.1:
    xorl    %r14d, %r14d
    xorl    %eax, %eax
    movl    %ebx, %edi
    callq   _init_stuff
    testl   %eax, %eax
    je  LBB0_5
## %bb.2:
    xorl    %r14d, %r14d
    xorl    %eax, %eax
    movl    %ebx, %edi
    callq   _prepare_stuff
    testl   %eax, %eax
    je  LBB0_4
## %bb.3:
    xorl    %eax, %eax
    movl    %ebx, %edi
    callq   _do_the_thing
    movl    %eax, %r14d
LBB0_4:
    xorl    %eax, %eax
    callq   _cleanup_3
LBB0_5:
    xorl    %eax, %eax
    callq   _cleanup_2
LBB0_6:
    xorl    %eax, %eax
    callq   _cleanup_1
    movl    %r14d, %eax
    popq    %rbx
    popq    %r14
    popq    %rbp
    retq
    .cfi_endproc
                                        ## -- End function

.subsections_via_symbols

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

struct lnode *insert(char *data, int len, struct lnode *list) {
    struct lnode *p, *q;
    uint8_t good;
    struct {
            uint8_t alloc_node : 1;
            uint8_t alloc_str : 1;
    } cleanup = { 0, 0 };

    // allocate node.
    p = (struct lnode *)malloc(sizeof(struct lnode));
    good = cleanup.alloc_node = (p != NULL);

    // good? then allocate str
    if (good) {
            p->str = (char *)malloc(sizeof(char)*len);
            good = cleanup.alloc_str = (p->str != NULL);
    }

    // good? copy data
    if(good) {
            memcpy ( p->str, data, len );
    }

    // still good? insert in list
    if(good) {
            if(NULL == list) {
                    p->next = NULL;
                    list = p;
            } else {
                    q = list;
                    while(q->next != NULL && good) {
                            // duplicate found--not good
                            good = (strcmp(q->str,p->str) != 0);
                            q = q->next;
                    }
                    if (good) {
                            p->next = q->next;
                            q->next = p;
                    }
            }
    }

    // not-good? cleanup.
    if(!good) {
            if(cleanup.alloc_str)   free(p->str);
            if(cleanup.alloc_node)  free(p);
    }

    // good? return list or else return NULL
    return (good? list: NULL);

}

источник: http://blog.staila.com/?p=114

Мы используем Daynix CSteps библиотеку как еще одно решение " проблемы перехода " в функциях инициализации.
Смотрите здесь и здесь .