Uso válido de goto para gerenciamento de erros em C?

Esta questão é na verdade o resultado de uma discussão interessante em programming.reddit.com há algum tempo. Basicamente, ele se resume ao seguinte código:

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;
}

O uso de gotohere parece ser o melhor caminho a seguir, resultando no código mais limpo e eficiente de todas as possibilidades, ou pelo menos é o que me parece. Citando Steve McConnell em Code Complete :

O goto é útil em uma rotina que aloca recursos, executa operações nesses recursos e, em seguida, desaloca os recursos. Com um goto, você pode limpar em uma seção do código. O goto reduz a probabilidade de você se esquecer de desalocar os recursos em cada lugar em que detectar um erro.

Outro suporte para essa abordagem vem do livro Linux Device Drivers , nesta seção .

O que você acha? Este caso é um uso válido para gotoem C? Você prefere outros métodos, que produzem código mais complicado e / ou menos eficiente, mas evita goto?

FWIF, acho que a linguagem de tratamento de erros que você deu no exemplo da pergunta é mais legível e fácil de entender do que qualquer uma das alternativas fornecidas nas respostas até agora. Embora gotoseja uma má ideia em geral, pode ser útil para o tratamento de erros quando feito de maneira simples e uniforme. Nessa situação, embora seja um goto, está sendo usado de forma bem definida e mais ou menos estruturada.

Como regra geral, evitar goto é uma boa ideia, mas os abusos que prevaleciam quando Dijkstra escreveu 'GOTO Considered Harmful' nem passam pela cabeça da maioria das pessoas como uma opção atualmente.

O que você delineou é uma solução generalizável para o problema de tratamento de erros - está bom para mim, desde que seja usada com cuidado.

Seu exemplo particular pode ser simplificado da seguinte forma (etapa 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;
}

Continuando o processo:

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;
}

Isso é, acredito, equivalente ao código original. Isso parece particularmente limpo, pois o próprio código original era muito limpo e bem organizado. Freqüentemente, os fragmentos de código não são tão organizados assim (embora eu aceite o argumento de que deveriam ser); por exemplo, freqüentemente há mais estado para passar para as rotinas de inicialização (configuração) do que mostrado e, portanto, mais estado para passar para as rotinas de limpeza também.

Estou surpreso que ninguém tenha sugerido essa alternativa, então, embora a questão já esteja por aí, vou acrescentá-la: uma boa maneira de resolver esse problema é usar variáveis ​​para rastrear o estado atual. Essa é uma técnica que pode ser usada, quer seja usada ou não gotopara chegar ao código de limpeza. Como qualquer técnica de codificação, ela tem prós e contras e não é adequada para todas as situações, mas se você estiver escolhendo um estilo, vale a pena considerar - especialmente se você quiser evitar gotosem terminar com ifs profundamente aninhados .

A ideia básica é que, para cada ação de limpeza que precise ser realizada, existe uma variável de cujo valor podemos dizer se a limpeza precisa ser executada ou não.

Vou mostrar a gotoversão primeiro, porque está mais próxima do código da pergunta original.

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;
}

Uma vantagem disso sobre algumas das outras técnicas é que, se a ordem das funções de inicialização for alterada, a limpeza correta ainda acontecerá - por exemplo, usando o switchmétodo descrito em outra resposta, se a ordem de inicialização mudar, então o switchdeve ser editado com muito cuidado para evitar tentar limpar algo que não foi inicializado em primeiro lugar.

Agora, alguns podem argumentar que esse método adiciona muitas variáveis ​​extras - e de fato, neste caso isso é verdade - mas na prática, muitas vezes uma variável existente já rastreia, ou pode ser feita para rastrear, o estado necessário. Por exemplo, se prepare_stuff()for realmente uma chamada para malloc(), ou para open(), então a variável que contém o ponteiro retornado ou o descritor de arquivo pode ser usada - por exemplo:

int fd = -1;

....

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

...

cleanup:

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

Agora, se rastrearmos adicionalmente o status de erro com uma variável, podemos evitar gotototalmente, e ainda limpar corretamente, sem ter um recuo que fica cada vez mais profundo quanto mais inicialização precisamos:

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;
}

Mais uma vez, existem potenciais críticas a isto:

• Todos aqueles "se" não prejudicam o desempenho? Não - porque no caso de sucesso, você tem que fazer todas as verificações de qualquer maneira (caso contrário, você não verificará todos os casos de erro); e, no caso de falha, a maioria dos compiladores otimizará a sequência de if (oksofar)verificações com falha para um único salto para o código de limpeza (o GCC certamente o faz) - e em qualquer caso, o caso de erro geralmente é menos crítico para o desempenho.

• Isso não é adicionar mais uma variável? Nesse caso, sim, mas muitas vezes a return_valuevariável pode ser usada para desempenhar o papel que oksofarestá desempenhando aqui. Se você estruturar suas funções para retornar erros de maneira consistente, poderá até evitar o segundo ifem cada caso:

  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);
  }

  Uma das vantagens de codificar assim é que a consistência significa que qualquer lugar onde o programador original se esqueceu de verificar o valor de retorno se destaca como um polegar dolorido, tornando muito mais fácil encontrar (aquela classe de) bugs.

Portanto - este é (ainda) mais um estilo que pode ser usado para resolver este problema. Usado corretamente, ele permite um código muito limpo e consistente - e, como qualquer técnica, nas mãos erradas pode acabar produzindo um código prolixo e confuso :-)

O problema com a gotopalavra-chave é geralmente mal compreendido. Não é totalmente mau. Você só precisa estar ciente dos caminhos de controle extras que você cria a cada goto. Torna-se difícil raciocinar sobre seu código e, portanto, sua validade.

FWIW, se você consultar os tutoriais do developer.apple.com, eles usarão a abordagem goto para tratamento de erros.

Não usamos gotos. Uma importância maior é colocada nos valores de retorno. O tratamento de exceções é feito por meio de setjmp/longjmp- o pouco que você puder.

Não há nada moralmente errado sobre a instrução goto mais do que há algo moralmente errado com (vazio) * ponteiros.

Tudo depende de como você usa a ferramenta. No caso (trivial) que você apresentou, uma instrução case pode atingir a mesma lógica, embora com mais overhead. A verdadeira questão é: "qual é o meu requisito de velocidade?"

goto é simplesmente rápido, especialmente se você tiver o cuidado de se certificar de que compila em um salto curto. Perfeito para aplicações onde a velocidade é um prêmio. Para outros aplicativos, provavelmente faz sentido levar a sobrecarga com if / else + case para manutenção.

Lembre-se: goto não mata aplicativos, os desenvolvedores matam aplicativos.

ATUALIZAÇÃO: Aqui está o exemplo de caso

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 é útil. É algo que seu processador pode fazer e é por isso que você deve ter acesso a ele.

Às vezes, você deseja adicionar algo à sua função e ir para simples permite que você faça isso facilmente. Isso pode economizar tempo ..

Em geral, eu consideraria o fato de que um trecho de código poderia ser escrito de forma mais clara usando gotocomo um sintoma de que o fluxo do programa é provavelmente mais complicado do que o geralmente desejável. Combinar outras estruturas de programa de maneiras estranhas para evitar o uso de gototentaria tratar o sintoma, em vez da doença. Seu exemplo específico pode não ser muito difícil de implementar sem goto:

  Faz {
    .. configurar o thing1 que precisará de limpeza apenas no caso de saída antecipada
    se (erro) quebrar;
    Faz
    {
      .. configurar o thing2 que precisará de limpeza em caso de saída antecipada
      se (erro) quebrar;
      // ***** VEJA O TEXTO SOBRE ESTA LINHA
    } enquanto (0);
    .. cleanup thing2;
  } enquanto (0);
  .. cleanup thing1;

mas se a limpeza só aconteceria quando a função falhasse, o gotocaso poderia ser tratado colocando um returnlogo antes do primeiro rótulo de destino. O código acima requer a adição de um returnna linha marcada com *****.

No cenário de "limpeza mesmo em caso normal", consideraria o uso de gotocomo sendo mais claro do que os construtos do/ while(0), entre outras coisas porque os próprios rótulos de destino gritam "OLHE PARA MIM" muito mais do que os construtos breake do/ while(0). Para o caso de "limpeza apenas se houver erro", a returninstrução acaba tendo que estar no pior lugar possível do ponto de vista da legibilidade (as instruções de retorno geralmente devem estar no início de uma função ou então no que "parece" o fim); ter um returnlogo antes de um rótulo de destino atende a essa qualificação muito mais prontamente do que ter um logo antes do final de um "loop".

BTW, um cenário onde às vezes uso gotopara tratamento de erros é dentro de uma switchinstrução, quando o código para vários casos compartilha o mesmo código de erro. Mesmo que meu compilador muitas vezes seja inteligente o suficiente para reconhecer que vários casos terminam com o mesmo código, acho que é mais claro dizer:

 REPARSE_PACKET:
  switch (pacote [0])
  {
    case PKT_THIS_OPERATION:
      if (condição problemática)
        goto PACKET_ERROR;
      ... lidar com THIS_OPERATION
      pausa;
    case PKT_THAT_OPERATION:
      if (condição problemática)
        goto PACKET_ERROR;
      ... lidar com THAT_OPERATION
      pausa;
    ...
    case PKT_PROCESS_CONDITIONALLY
      if (packet_length <9)
        goto PACKET_ERROR;
      if (condição_pacote envolvendo pacote [4])
      {
        comprimento_do_pacote - = 5;
        memmove (pacote, pacote + 5, comprimento_do_pacote);
        goto REPARSE_PACKET;
      }
      outro
      {
        pacote [0] = PKT_CONDITION_SKIPPED;
        pacote [4] = comprimento_do_pacote;
        comprimento_do_pacote = 5;
        packet_status = READY_TO_SEND;
      }
      pausa;
    ...
    padrão:
    {
     PACKET_ERROR:
      packet_error_count ++;
      comprimento_do_pacote = 4;
      pacote [0] = PKT_ERROR;
      packet_status = READY_TO_SEND;
      pausa;
    }
  }   

Embora seja possível substituir as gotoinstruções por {handle_error(); break;}, e embora seja possível usar um do/ while(0)loop junto com continuepara processar o pacote de execução condicional encapsulado, realmente não acho que isso seja mais claro do que usar um goto. Além disso, embora possa ser possível copiar o código de PACKET_ERRORtodos os lugares em que goto PACKET_ERRORé usado, e enquanto um compilador pode escrever o código duplicado uma vez e substituir a maioria das ocorrências com um salto para essa cópia compartilhada, o uso de gototorna mais fácil notar os locais que configuram o pacote de maneira um pouco diferente (por exemplo, se a instrução "executar condicionalmente" decidir não executar).

Eu, pessoalmente, sou um seguidor do "Poder do Dez - 10 Regras para Escrever Código de Segurança Crítica" .

Vou incluir um pequeno trecho desse texto que ilustra o que acredito ser uma boa ideia sobre goto.

Regra: Restrinja todo o código a construções de fluxo de controle muito simples - não use instruções goto, construções setjmp ou longjmp e recursão direta ou indireta.

Justificativa: Fluxo de controle mais simples se traduz em recursos mais fortes para verificação e geralmente resulta em clareza de código aprimorada. O banimento da recursão é talvez a maior surpresa aqui. Sem recursão, entretanto, temos a garantia de ter um gráfico de chamada de função acíclica, que pode ser explorado por analisadores de código e pode ajudar diretamente a provar que todas as execuções que deveriam ser limitadas são de fato limitadas. (Observe que essa regra não exige que todas as funções tenham um único ponto de retorno - embora isso também simplifique o fluxo de controle. Há casos suficientes, porém, em que um retorno de erro antecipado é a solução mais simples.)

Banir o uso de goto parece ruim, mas:

Se as regras parecem draconianas à primeira vista, tenha em mente que elas se destinam a possibilitar a verificação de códigos onde literalmente sua vida pode depender de sua correção: código que é usado para controlar o avião em que você voa, a usina nuclear a alguns quilômetros de onde você mora, ou da espaçonave que carrega astronautas para a órbita. As regras funcionam como o cinto de segurança do seu carro: no início, talvez sejam um pouco desconfortáveis, mas depois de um tempo seu uso torna-se natural e não usá-las torna-se inimaginável.

Concordo que a limpeza goto na ordem reversa fornecida na pergunta é a maneira mais limpa de limpar as coisas na maioria das funções. Mas também queria salientar que, às vezes, você deseja que sua função seja limpa de qualquer maneira. Nesses casos, uso a seguinte variante if if (0) {label:} idiom para ir ao ponto certo do processo de limpeza:

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 ;
 }

Parece-me que cleanup_3deveria fazer a sua limpeza, então ligue cleanup_2. Da mesma forma, cleanup_2deve fazer a limpeza e, em seguida, chamar cleanup_1. Parece que sempre que você fizer cleanup_[n]isso, isso cleanup_[n-1]será necessário, portanto, deve ser responsabilidade do método (de modo que, por exemplo, cleanup_3nunca possa ser chamado sem chamar cleanup_2e possivelmente causar um vazamento).

Dada essa abordagem, em vez de gotos, você simplesmente chamaria a rotina de limpeza e retornaria.

A gotoabordagem não é errada ou ruim , porém, é importante notar que não é necessariamente a abordagem "mais limpa" (IMHO).

Se você está procurando o desempenho ideal, suponho que a gotosolução seja a melhor. Só espero que seja relevante, no entanto, em alguns poucos aplicativos de desempenho crítico (por exemplo, drivers de dispositivo, dispositivos incorporados, etc). Caso contrário, é uma microotimização que tem prioridade inferior à clareza do código.

Eu acho que a questão aqui é falaciosa com relação ao código fornecido.

Considerar:

1. do_something (), init_stuff () e prepare_stuff () parecem saber se falharam, pois retornam false ou nil nesse caso.
2. A responsabilidade por configurar o estado parece ser de responsabilidade dessas funções, uma vez que não há estado sendo configurado diretamente em foo ().

Portanto: do_something (), init_stuff () e prepare_stuff () devem estar fazendo sua própria limpeza . Ter uma função cleanup_1 () separada que limpa após do_something () quebra a filosofia de encapsulamento. É um design ruim.

Se eles fizeram sua própria limpeza, foo () se tornará bastante simples.

Por outro lado. Se foo () realmente criou seu próprio estado que precisava ser derrubado, então goto seria apropriado.

Aqui está o que eu preferi:

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;
}

Discussão antiga, no entanto ... que tal usar "anti-padrão de seta" e encapsular mais tarde cada nível aninhado em uma função inline estática? O código parece limpo, é ideal (quando as otimizações estão habilitadas) e nenhum goto é usado. Resumindo, divida e conquiste. Abaixo um exemplo:

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;
}

Em termos de espaço, estamos criando três vezes a variável na pilha, o que não é bom, mas isso desaparece ao compilar com -O2 removendo a variável da pilha e usando um registro neste exemplo simples. O que obtive do bloco acima gcc -S -O2 test.cfoi o seguinte:

    .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

Prefiro usar a técnica descrita no exemplo a seguir ...

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);

}

fonte: http://blog.staila.com/?p=114

Usamos a Daynix CStepsbiblioteca como outra solução para o " problema goto " nas funções init.
Veja aqui e aqui .