wake-up-neo.net

Gültige Verwendung von goto für die Fehlerverwaltung in C?

Diese Frage ist eigentlich das Ergebnis eines interessante Diskussion bei programming.reddit.com vor einiger Zeit. Im Grunde läuft es auf den folgenden Code:

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

Die Verwendung von goto scheint hier der beste Weg zu sein, was zu dem saubersten und effizientesten Code aller Möglichkeiten führt, oder zumindest scheint es mir so. Steve McConnell zitiert in Code Complete:

Der goto ist nützlich in einer Routine, die ordnet Ressourcen zu, führt .__ durch. Operationen an diesen Ressourcen und dann werden die Ressourcen freigegeben. Mit einer goto, Sie können in einem Abschnitt aufräumen des Codes. Das goto reduziert das Wahrscheinlichkeit, dass Sie das vergessen haben lege die Ressourcen an jedem Ort frei Sie entdecken einen Fehler.

Eine weitere Unterstützung für diesen Ansatz kommt aus dem Buch Linux Device Drivers in diesem Abschnitt .

Was denkst du? Ist dieser Fall eine gültige Verwendung für goto in C? Würden Sie andere Methoden bevorzugen, die mehr gewundenen und/oder weniger effizienten Code erzeugen, aber goto vermeiden?

84
Eli Bendersky

FWIF, ich finde das Fehlerbehandlungs-Idiom, das Sie im Beispiel der Frage angegeben haben, lesbarer und verständlicher als alle bisher in den Antworten angegebenen Alternativen. Obwohl goto im Allgemeinen eine schlechte Idee ist, kann es für die Fehlerbehandlung hilfreich sein, wenn dies auf einfache und einheitliche Weise geschieht. In dieser Situation, obwohl es sich um ein goto handelt, wird es klar und mehr oder weniger strukturiert verwendet.

53
Michael Burr

In der Regel ist das Vermeiden von Goto eine gute Idee, aber die Missbräuche, die vorherrschend waren, als Dijkstra zum ersten Mal "GOTO Considered Harmful" schrieb, treffen die meisten Menschen heutzutage nicht einmal als Option.

Was Sie skizzieren, ist eine verallgemeinerbare Lösung für das Fehlerbehandlungsproblem - es ist gut für mich, solange es sorgfältig verwendet wird.

Ihr spezielles Beispiel kann wie folgt vereinfacht werden (Schritt 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;
}

Fortsetzung des Prozesses:

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

Dies entspricht meines Erachtens dem ursprünglichen Code. Dies sieht besonders sauber aus, da der ursprüngliche Code selbst sehr sauber und gut organisiert war. Oft sind die Codefragmente nicht so aufgeräumt (obwohl ich ein Argument akzeptieren würde, das sie sein sollten); Zum Beispiel gibt es häufig mehr Zustände, die an die Initialisierungsroutinen (Setup-Routinen) übergeben werden können, als gezeigt, und damit mehr Zustände, die auch an die Bereinigungsroutinen übergeben werden können.

16

Ich bin überrascht, dass niemand diese Alternative vorgeschlagen hat, und obwohl ich die Frage schon eine Weile hatte, füge ich sie hinzu: Eine gute Möglichkeit, dieses Problem anzugehen, ist die Verwendung von Variablen, um den aktuellen Status zu verfolgen. Dies ist eine Technik, die verwendet werden kann, unabhängig davon, ob goto verwendet wird, um zum Bereinigungscode zu gelangen. Wie bei jeder Codierungsmethode gibt es Vor- und Nachteile und ist nicht für jede Situation geeignet. Wenn Sie sich für einen Stil entscheiden, ist dies eine Überlegung wert - insbesondere, wenn Sie goto vermeiden möchten, ohne bei tief verschachtelten ifs zu enden.

Die Grundidee ist, dass es für jede Bereinigungsaktion, die möglicherweise ausgeführt werden muss, eine Variable gibt, deren Wert wir erkennen können, ob die Bereinigung durchgeführt werden muss oder nicht.

Ich zeige zuerst die goto-Version, weil sie dem Code in der ursprünglichen Frage näher kommt.

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

Ein Vorteil gegenüber einigen anderen Techniken besteht darin, dass bei einer Änderung der Reihenfolge der Initialisierungsfunktionen immer noch die korrekte Bereinigung erfolgt - beispielsweise mithilfe der in einer anderen Antwort beschriebenen switch-Methode, falls sich die Reihenfolge der Initialisierung ändert Die switch muss sehr sorgfältig bearbeitet werden, um zu vermeiden, dass etwas bereinigt wird, das nicht wirklich initialisiert wurde.

Einige mögen nun argumentieren, dass diese Methode eine ganze Menge zusätzlicher Variablen hinzufügt - und dies ist in diesem Fall tatsächlich der Fall -, aber in der Praxis verfolgt eine vorhandene Variable oft bereits den erforderlichen Status oder kann dazu gemacht werden, den erforderlichen Status zu verfolgen. Wenn zum Beispiel prepare_stuff() tatsächlich ein Aufruf an malloc() oder an open() ist, kann die Variable verwendet werden, die den zurückgegebenen Zeiger oder Dateideskriptor enthält - zum Beispiel:

int fd = -1;

....

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

...

cleanup:

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

Wenn wir jetzt zusätzlich den Fehlerstatus mit einer Variablen verfolgen, können wir goto vollständig vermeiden und trotzdem korrekt bereinigen, ohne dass die Einrückung tiefer und tiefer wird, je mehr Initialisierung wir benötigen:

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

Auch hier gibt es potentielle Kritikpunkte:

  • Nicht alle diese "wenn" verletzen die Leistung? Nein, denn im Erfolgsfall müssen Sie trotzdem alle Prüfungen durchführen (andernfalls prüfen Sie nicht alle Fehlerfälle). Im Fehlerfall optimieren die meisten Compiler die Reihenfolge der fehlgeschlagenen if (oksofar)-Überprüfungen mit einem einzigen Sprung zum Bereinigungscode (GCC tut dies natürlich) - und in jedem Fall ist der Fehlerfall normalerweise weniger kritisch für die Leistung.
  • Fügt das nicht noch eine weitere Variable hinzu? In diesem Fall ja, aber oft kann die Variable return_value verwendet werden, um die Rolle zu spielen, die oksofar hier spielt. Wenn Sie Ihre Funktionen so strukturieren, dass Fehler konsistent zurückgegeben werden, können Sie sogar jeweils die zweite if vermeiden:

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

    Einer der Vorteile einer solchen Codierung ist, dass die Konsistenz bedeutet, dass jeder Ort, an dem der ursprüngliche Programmierer vergessen hat, den Rückgabewert zu überprüfen, wie ein wunder Daumen ragt, was es sehr viel einfacher macht, diese eine Klasse von Fehlern zu finden.

Dies ist also (noch) ein weiterer Stil, mit dem dieses Problem gelöst werden kann. Bei korrekter Verwendung kann sehr sauberer, konsistenter Code verwendet werden. Wie bei jeder Technik kann es in den falschen Händen dazu führen, dass Code erzeugt wird, der langwierig und verwirrend ist :-)

15
psmears

Das Problem mit dem Schlüsselwort goto wird größtenteils missverstanden. Es ist nicht einfach böse. Sie müssen nur die zusätzlichen Steuerpfade kennen, die Sie mit jedem Schritt erstellen. Es wird schwierig, über Ihren Code und damit seine Gültigkeit nachzudenken.

FWIW: Wenn Sie die Tutorials von developer.Apple.com nachschlagen, verwenden sie die goto-Methode zur Fehlerbehandlung. 

Wir verwenden keine gotos. Den Rückgabewerten wird eine höhere Bedeutung beigemessen. Die Ausnahmebehandlung erfolgt über setjmp/longjmp - was immer Sie können.

8
dirkgently

Es gibt nichts moralischeres an der goto-Anweisung, als etwas (moralisch) mit (leeren) * Zeigern.

Es ist alles darin, wie Sie das Tool verwenden. In dem (trivialen) Fall, den Sie vorgestellt haben, kann eine Case-Anweisung dieselbe Logik erreichen, allerdings mit mehr Aufwand. Die eigentliche Frage ist: "Was ist meine Geschwindigkeitsanforderung?" 

goto ist einfach schnell, besonders wenn Sie darauf achten, dass es zu einem kurzen Sprung kompiliert wird. Perfekt für Anwendungen, bei denen Geschwindigkeit eine Prämie ist. Für andere Anwendungen ist es wahrscheinlich sinnvoll, den Overhead-Treffer mit if/else + zu berücksichtigen, um die Wartbarkeit zu gewährleisten.

Denken Sie daran: goto beendet keine Anwendungen, Entwickler töten Anwendungen.

UPDATE: Hier ist das Fallbeispiel

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

GOTO ist nützlich. Es ist etwas, was Ihr Prozessor tun kann, und deshalb sollten Sie darauf Zugriff haben.

Manchmal möchten Sie etwas zu Ihrer Funktion hinzufügen, und Sie können das einfach tun, indem Sie einfach gehen ... Es kann Zeit sparen.

2
toto

Im Allgemeinen würde ich die Tatsache, dass ein Stück Code mit goto am klarsten geschrieben werden könnte, als ein symptom dass der Programmablauf wahrscheinlich komplizierter ist als im Allgemeinen wünschenswert. Die Kombination anderer Programmstrukturen auf ungewöhnliche Weise, um die Verwendung von goto zu vermeiden, würde versuchen, das Symptom und nicht die Krankheit zu behandeln. Ihr spezielles Beispiel könnte ohne goto nicht allzu schwierig zu implementieren sein:

 tun {
 .. Richten Sie thing1 ein, das nur im Falle eines vorzeitigen Austritts bereinigt werden muss 
 Wenn (Fehler) Bruch; 
 tun
 {
 .. Richten Sie thing2 ein, das im Falle eines vorzeitigen Austritts bereinigt werden muss 
 Wenn (Fehler) Bruch; 
 // ***** TEXT ZU DIESER LINIE SEHEN 
 } while (0); 
 .. cleanup thing2; 
 } while (0); 
 .. cleanup thing1; 

wenn die Bereinigung jedoch nur dann durchgeführt werden sollte, wenn die Funktion fehlgeschlagen ist, kann der Fall goto behandelt werden, indem ein return direkt vor der ersten Zielbezeichnung platziert wird. Der obige Code erfordert, dass an der mit ***** markierten Zeile ein return hinzugefügt wird.

Im Szenario "Bereinigung selbst im Normalfall" würde ich die Verwendung von goto als klarer betrachten als die Konstrukte do/while(0), da das Ziel selbst beschriftet "LOOK AT ME" als die Konstrukte break und do/while(0). Für den Fall "Bereinigung nur bei Fehlern" muss die return-Anweisung aus Sicht der Lesbarkeit an fast ungünstigster Stelle stehen (return-Anweisungen sollten im Allgemeinen entweder am Anfang einer Funktion stehen oder an was "aussieht") " das Ende); ein return hat, bevor ein Ziel-Label diese Qualifikation erfüllt, viel leichter als ein Label kurz vor dem Ende einer "Schleife".

Übrigens, ein Szenario, in dem ich manchmal goto für die Fehlerbehandlung verwende, ist in einer switch-Anweisung enthalten, wenn der Code für mehrere Fälle denselben Fehlercode verwendet. Obwohl mein Compiler oft klug genug wäre, um zu erkennen, dass mehrere Fälle mit demselben Code enden, ist es meiner Meinung nach klarer zu sagen:

 REPARSE_PACKET: 
 Schalter (Paket [0]) 
 {
 Fall PKT_THIS_OPERATION: 
 wenn (problembedingung) 
 gehe zu PACKET_ERROR; 
 ... behandelt THIS_OPERATION 
 brechen;
 Fall PKT_THAT_OPERATION: 
 wenn (problembedingung) 
 gehe zu PACKET_ERROR; 
 ... behandelt THAT_OPERATION 
 brechen;
 ...
 case PKT_PROCESS_CONDITIONALLY 
 if (packet_length <9) 
 gehe zu PACKET_ERROR; 
 if (paket_bedingung mit paket [4]) 
 {
 Paketlänge - = 5; 
 memmove (Paket, Paket + 5, Paketlänge); 
 gehe zu REPARSE_PACKET; 
 } 
 sonst
 {
 Paket [0] = PKT_CONDITION_SKIPPED; 
 Paket [4] = Paketlänge; 
 Paketlänge = 5; 
 packet_status = READY_TO_SEND; 
 } 
 brechen;
 ...
 Standard:
 {
 PACKET_ERROR: 
 packet_error_count ++; 
 Paketlänge = 4; 
 Paket [0] = PKT_ERROR; 
 packet_status = READY_TO_SEND; 
 brechen;
 } 
 } 

Obwohl man die goto-Anweisungen durch {handle_error(); break;} ersetzen könnte, und obwohl man eine do/while(0)-Schleife zusammen mit continue verwenden könnte, um das umschlossene Conditional-Execute-Paket zu verarbeiten, glaube ich nicht, dass dies klarer ist als die Verwendung eines goto. Während es möglich ist, den Code aus PACKET_ERROR überall zu kopieren, wo goto PACKET_ERROR verwendet wird, und während ein Compiler den duplizierten Code einmal ausschreibt und die meisten Vorkommnisse durch einen Sprung zu dieser gemeinsam genutzten Kopie ersetzt, macht es die Verwendung von goto einfacher Stellen zu bemerken, die das Paket ein wenig anders einrichten (z. B. wenn der Befehl "bedingt ausführen" entscheidet, nicht auszuführen).

1
supercat

Ich bin damit einverstanden, dass die in der Frage angegebene umgekehrte Reihenfolge der Aufräumarbeiten die sauberste Art ist, Dinge in den meisten Funktionen aufzuräumen. Ich wollte aber auch darauf hinweisen, dass Ihre Funktion manchmal aufgeräumt werden soll. In diesen Fällen verwende ich die folgende Variante, wenn if (0) {label:} idiom, um zum richtigen Punkt des Bereinigungsprozesses zu gelangen:

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

Ich persönlich bin ein Anhänger der "Zehnerpotenz - 10 Regeln für das Schreiben von sicherheitskritischem Code" .

Ich werde einen kleinen Ausschnitt aus diesem Text beifügen, der zeigt, was ich für eine gute Idee von goto halte.


Regel: Beschränken Sie den gesamten Code auf sehr einfache Kontrollflusskonstrukte. Verwenden Sie keine goto-Anweisungen, setjmp- oder longjmp-Konstrukte und keine direkte oder indirekte Rekursion.

Begründung: Ein einfacherer Kontrollfluss führt zu besseren Überprüfungsmöglichkeiten und häufig zu einer besseren Code-Klarheit. Die Verbannung der Rekursion ist hier vielleicht die größte Überraschung. Ohne Rekursion wird jedoch garantiert, dass wir einen azyklischen Funktionsaufruf-Graphen haben, der von Codeanalysatoren ausgenutzt werden kann und direkt dazu beiträgt, zu beweisen, dass alle Ausführungen, die eingeschränkt werden sollten, tatsächlich beschränkt sind. (Beachten Sie, dass diese Regel nicht erfordert, dass alle Funktionen über einen einzigen Rückgabepunkt verfügen. Dies vereinfacht jedoch häufig auch den Kontrollfluss. Es gibt jedoch genügend Fälle, in denen eine frühe Fehlerrückgabe die einfachere Lösung ist.)


Das Verbannen der Verwendung von goto scheint schlecht zu sein, aber:

Wenn die Regeln auf den ersten Blick drakonisch erscheinen, denken Sie daran, dass sie es ermöglichen sollen, Code zu überprüfen, wenn Ihr Leben buchstäblich von seiner Richtigkeit abhängt : Code, der verwendet wird, um das Flugzeug zu steuern, mit dem Sie fliegen, das Kernkraftwerk wenige Kilometer von Ihrem Wohnort entfernt oder das Raumschiff, das Astronauten in die Umlaufbahn befördert. Die Regeln wirken wie der Sicherheitsgurt in Ihrem Auto: Zunächst sind sie vielleicht etwas unangenehm, aber nach einer Weile wird ihre Verwendung zur Selbstverständlichkeit und ihre Nichtbenutzung unvorstellbar.

1

Folgendes habe ich bevorzugt:

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;
}
0
Mikko Korkalo

Scheint mir, dass cleanup_3 seine Bereinigung durchführen sollte, dann rufen Sie cleanup_2 auf. In ähnlicher Weise sollte cleanup_2 die Bereinigung durchführen und dann Bereinigung_1 aufrufen. Es scheint, dass jedes Mal, wenn Sie cleanup_[n] ausführen, cleanup_[n-1] erforderlich ist, daher sollte es in der Verantwortung der Methode liegen (z. B. kann cleanup_3 nie aufgerufen werden, ohne cleanup_2 zu rufen und möglicherweise ein Leck zu verursachen.)

Bei diesem Ansatz würden Sie anstelle von gotos einfach die Bereinigungsroutine aufrufen und dann zurückkehren.

Der goto-Ansatz ist nicht falsch oder schlecht , aber es ist nur erwähnenswert, dass es nicht unbedingt der "sauberste" Ansatz (IMHO) ist.

Wenn Sie nach der optimalen Leistung suchen, nehme ich an, dass die goto-Lösung die beste ist. Ich erwarte nur, dass dies für einige wenige, leistungskritische Anwendungen (z. B. Gerätetreiber, eingebettete Geräte usw.) relevant ist. Ansonsten ist es eine Mikrooptimierung, die eine niedrigere Priorität hat als die Klarheit des Codes.

0
Ryan Emerle

Ich denke, dass die Frage hier in Bezug auf den gegebenen Code falsch ist.

Erwägen:

  1. do_something (), init_stuff () und prepar_stuff () scheinen zu wissen, ob sie fehlgeschlagen sind, da sie in diesem Fall entweder false oder nil zurückgeben.
  2. Die Zuständigkeit für das Einrichten eines Zustands scheint die Zuständigkeit dieser Funktionen zu sein, da in foo () kein Zustand direkt eingerichtet wird.

Deshalb: do_something (), init_stuff () und prepar_stuff () sollten ihre eigene Bereinigung durchführen . Eine separate cleanup_1 () - Funktion, die nach do_something () aufräumt, unterbricht die Philosophie der Einkapselung. Es ist schlechtes Design.

Wenn sie ihre eigene Bereinigung durchgeführt haben, wird foo () ziemlich einfach.

Auf der anderen Seite. Wenn foo () tatsächlich einen eigenen Staat geschaffen hat, der abgerissen werden musste, wäre goto angemessen.

0
Simon Woodside