Допустимое использование goto для управления ошибками в C?
Этот вопрос на самом деле является результатом интересного обсуждения на сайте program.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;
}
Использование goto здесь, кажется, лучший путь, в результате чего получается самый чистый и эффективный код из всех возможных, или, по крайней мере, мне так кажется. Цитирование Стива Макконнелла в коде завершено:
Goto полезно в подпрограмме, которая распределяет ресурсы, выполняет операции с этими ресурсами, а затем освобождает ресурсы. С помощью goto вы можете выполнить очистку в одном разделе кода. Goto снижает вероятность того, что вы забудете освободить ресурсы в каждом месте, где вы обнаружите ошибку.
Другая поддержка этого подхода содержится в книге " Драйверы устройств Linux" в этом разделе.
Как вы думаете? Является ли этот случай допустимым для использования goto в С? Вы бы предпочли другие методы, которые производят более замысловатый и / или менее эффективный код, но избегают goto?
16 ответов
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 Ключевое слово в основном неправильно понято. Это не просто зло. Вам просто нужно знать о дополнительных путях управления, которые вы создаете при каждом переходе. Становится трудно рассуждать о вашем коде и, следовательно, о его достоверности.
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:
делать {
.. настроить thing1, которая будет нуждаться в очистке только в случае досрочного выхода
если (ошибка) перерыв;
делать
{
.. настроить thing2, которая будет нуждаться в очистке в случае досрочного выхода
если (ошибка) перерыв;
// ***** ПОСМОТРЕТЬ ТЕКСТ ОТНОСИТЕЛЬНО ЭТОЙ ЛИНИИ
} while(0);
.. уборка вещь2;
} while(0);
.. уборка вещь1;но если очистка должна была произойти только в случае сбоя функции, goto дело может быть решено путем помещения return как раз перед первой целевой меткой. Приведенный выше код потребует добавления return на линии, отмеченной *****,
В сценарии "очистки даже в обычном случае" я бы рассматривал использование goto как яснее, чем do/while(0) конструкции, среди прочего, потому что сами метки назначения практически выкрикивают "LOOK AT ME" гораздо больше, чем break а также do/while(0) строит. В случае "очистки только в случае ошибки" return оператор заканчивается тем, что должен быть в самом худшем из возможных мест с точки зрения читабельности (операторы возврата обычно должны быть либо в начале функции, либо в том, что "похоже" на конец); иметь return незадолго до того, как целевой ярлык встречает эту квалификацию гораздо легче, чем иметь его непосредственно перед концом "цикла".
Кстати, один сценарий, где я иногда использую goto для обработки ошибок находится в пределах switch утверждение, когда код для нескольких случаев имеет один и тот же код ошибки. Несмотря на то, что мой компилятор часто был достаточно умен, чтобы распознать, что несколько случаев заканчиваются одним и тем же кодом, я думаю, что проще сказать:
REPARSE_PACKET:
Переключатель (пакет [0])
{
case PKT_THIS_OPERATION:
если (условие проблемы)
перейти к PACKET_ERROR;
... справиться с этим
перерыв;
case PKT_THAT_OPERATION:
если (условие проблемы)
перейти к PACKET_ERROR;
... обрабатывать THAT_OPERATION
перерыв;
...
case PKT_PROCESS_CONDITIONALLY
если (длина_пакета < 9)
перейти к PACKET_ERROR;
if (условие_пакета с участием пакета [4])
{
длина_пакета -= 5;
memmove(пакет, пакет +5, длина_пакета);
перейти к 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 облегчает обнаружение мест, которые устанавливают пакет немного по-другому (например, если команда "выполнить условно" решает не выполнять).
Я согласен с тем, что очистка 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 ;
}
Я лично являюсь последователем "Сила десяти - 10 правил для написания критического кода безопасности".
Я включу небольшой фрагмент этого текста, который иллюстрирует то, что я считаю хорошей идеей относительно goto.
Правило: ограничьте весь код очень простыми конструкциями потока управления - не используйте операторы goto, конструкции setjmp или longjmp и прямую или косвенную рекурсию.
Обоснование: упрощенный поток управления приводит к расширению возможностей проверки и часто приводит к повышению четкости кода. Изгнание рекурсии, пожалуй, самый большой сюрприз здесь. Однако без рекурсии у нас гарантированно будет график вызовов ациклических функций, который может быть использован анализаторами кода и может непосредственно помочь доказать, что все исполнения, которые должны быть ограничены, на самом деле ограничены. (Обратите внимание, что это правило не требует, чтобы все функции имели единую точку возврата - хотя это часто также упрощает поток управления. Однако существует достаточно случаев, когда досрочное возвращение ошибки является более простым решением.)
Изгнание goto кажется плохим, но:
Если правила на первый взгляд кажутся драконовскими, имейте в виду, что они предназначены для того, чтобы сделать возможным проверку кода, в котором буквально ваша жизнь может зависеть от его правильности: код, который используется для управления самолетом, на котором вы летите, атомная электростанция в нескольких милях от того места, где вы живете, или космического корабля, который доставляет астронавтов на орбиту. Правила действуют подобно ремню безопасности в вашей машине: вначале они, возможно, немного неудобны, но через некоторое время их использование становится второстепенным, и неиспользование их становится невообразимым.
Я думаю, что вопрос здесь ошибочен в отношении данного кода.
Рассматривать:
- do_something (), init_stuff () и prepare_stuff (), кажется, знают, что они потерпели неудачу, так как они возвращают либо false, либо nil в этом случае.
- Ответственность за настройку состояния, по-видимому, лежит на этих функциях, поскольку в 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;
}
Да, это действительный и лучший метод для исключений в C. Весь механизм обработки ошибок любого языка просто перескакивает от ошибки к обработке, как переход к метке. Но рассмотрите возможность размещения метки после goto в потоке выполнения и в той же области.
Старое обсуждение, однако... как насчет использования "антипаттерна стрелок" и инкапсуляции позже каждого вложенного уровня в статическую встроенную функцию? Код выглядит чистым, оптимальным (при включенной оптимизации), 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
Мне кажется что cleanup_3 должен сделать свою уборку, затем позвонить cleanup_2, Так же, cleanup_2 следует выполнить очистку, затем вызвать cleanup_1. Похоже, что в любое время вы делаете cleanup_[n], тот cleanup_[n-1] требуется, таким образом, это должно быть ответственностью метода (так что, например, cleanup_3 нельзя звонить без звонка cleanup_2 и, возможно, вызывает утечку.)
При таком подходе вместо gotos вы просто вызываете процедуру очистки, а затем возвращаетесь.
goto подход не является неправильным или плохим, однако, просто стоит отметить, что это не обязательно самый "чистый" подход (ИМХО).
Если вы ищете оптимальную производительность, то я полагаю, что goto Решение лучшее. Однако я ожидаю, что это будет актуально только в некоторых, критически важных для производительности приложениях (например, драйверы устройств, встроенные устройства и т. Д.). В противном случае, это микрооптимизация, которая имеет более низкий приоритет, чем ясность кода.
Я предпочитаю использовать технику, описанную в следующем примере...
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