goto is evil



  • Folgendes Konstrukt:

    again:
      rc = slow_syscall();
      if (rc < 0) {
        if (errno == EINTR) {
          if (restart)
            goto again;
        } else {
          perror("slow_syscall");
          exit(EXIT_FAILURE);
        }
      }
    

    Ich hätt gern von den goto-is-evil-Vertretern eine schönheitspreisverdächtige Umschreibung davon ohne goto. Lösungen mit höchstens einem "versteckten" goto kriegen Zusatzpunkte, solche, die sich an die Prinzipien der strukturierten Programmierung halten, nochmehr. Lösungen, die nicht lesbar sind, kriegen Abzug. Das Kriterium dabei lautet, dass ein Unix-Programmierer sofort sehen soll, was dort passiert, also einfache Äquivalenz des Ablaufs reicht nicht.

    Für die Unix-Unbeleckten:

    • Systemaufrufe geben bei Fehlern normalerweise einen Wert < 0 zurück, meistens -1.
    • "Langsame" Systemaufrufe werden bei Empfang eines Signals unterbrochen, errno wird auf EINTR gesetzt. Es gibt dann, so man nichts anderes vorhat (-> restart-Flag), nur die Möglichkeit, den Systemaufruf einfach neuzustarten ("schnelle" Systemaufrufe werden automatisch neugestartet.)


  • darf die Lösung in C++ sein?



  • Lösungen mit höchstens einem "versteckten" goto kriegen Zusatzpunkte

    do
      rc = slow_syscall();
      if (rc < 0) {
        if (errno == EINTR) {
          if (restart)
            continue;
        } else {
          perror("slow_syscall");
          exit(EXIT_FAILURE);
        }
      }
    while(false);
    

    krieg ich jetzt nen preis?



  • do {
       rc = slow_syscall();
    } while (rc < 0 && errno == EINTR && restart);
    
    if(rc < 0 && errno != EINTR) {
       perror("slow_syscall");
       exit(EXIT_FAILURE);
    }
    


  • for( ;restart != false; rc = slow_syscall())
    {
      if(rc<0){
         if(errno == EINTR){
           }
         else
           {
            perror("slow_syscall");
            exit(EXIT_FAILURE)
           }
       }
    }
    

    Wieviel Punkte ?

    [ Dieser Beitrag wurde am 14.11.2002 um 13:29 Uhr von devil81 editiert. ]



  • edit: fehler, muß den erstmal wegmachen.
    edit: und zurückhalten bis heut abend um 5, erstmal zugucken, obs einer als einzeiler schafft.

    [ Dieser Beitrag wurde am 14.11.2002 um 13:56 Uhr von volkard editiert. ]



  • do
    {
      rc = slow_syscall();
      if(!(rx < 0) && !(errno == EINTR) && !restart)
      {
        perror("slow_syscall");
        exit(EXIT_FAILURE);
      }
    }while(true)
    


  • while(!restart) {
        if (((rc=slow_syscall()) < 0) && (errno==EINTR))
                continue;
        break;
    }
    

    Edit: Kleine Änderung gemacht ...

    [ Dieser Beitrag wurde am 14.11.2002 um 15:48 Uhr von CengizS editiert. ]



  • rc = 0;
    while( (rc = slow_syscall()) < 0 && errno == EINTR && restart );
    if( rc < 0 ) {
        perror("slow_syscall");
        exit(EXIT_FAILURE);
    }
    


  • Original erstellt von DrGreenthumb:
    **```cpp
    rc = 0;
    while( (rc = slow_syscall()) < 0 && errno == EINTR && restart );
    if( rc < 0 ) {
    perror("slow_syscall");
    exit(EXIT_FAILURE);
    }

    /* Mist.. is ja fast dasselbe wie von DrZoidberg */



  • da exit das Programm beendet, sollte dies äquivalent sein:

    do
    {
      if ((rc = slow_syscall()) >= 0) break;
      if (errno != EINTR)
      {
        perror("slow_syscall");
        exit(EXIT_FAILURE);
      }
    } while (restart);
    

    Oder so:

    while ((rc = slow_syscall())<0 && errno==EINTR && restart);
    if (rc < 0 && errno != EINTR)
    {
      perror("slow_syscall");
      exit(EXIT_FAILURE);
    }
    

    Die erste Lösung gefällt mir persönlich besser.
    Das ganze 'Kapitel' wird durch do-while schön gekapselt, die Vergleiche
    kommen nicht doppelt vor.
    Das 'early-out' mit dem break ist eine einfache Anweisung,
    das Loopkriterium offensichtlicher.

    [ Dieser Beitrag wurde am 14.11.2002 um 16:12 Uhr von Bitsy editiert. ]



  • wie sieht der code aus, den ihr gerne verwenden würdet? würde einer von euch gerne ne lösung verwenden von den hier genannten? oder was nehmt ihr sonst?



  • Ich würds bei dem goto belassen.



  • wie sieht der code aus, den ihr gerne verwenden würdet? würde einer von euch gerne ne lösung verwenden von den hier genannten? oder was nehmt ihr sonst?

    Einen kontrollierten Buffer-Overflow 😉

    mfg
    v R



  • @volkard:
    Von der Form her würde ich meine erste Lösung vertreten (wieso steht ja dabei).
    Um ganz ehrlich zu sein, mag ich aber exit generell nicht in einer 'tiefen' Routine.
    Ich würde deshalb die Routine mit einem Fehlercode returnen, bzw. eine Exception werfen (wenn ich das endlich mal lernen würde).



  • Original erstellt von Bitsy:
    @volkard:
    Ich würde deshalb die Routine mit einem Fehlercode returnen, bzw. eine Exception werfen (wenn ich das endlich mal lernen würde).

    ok, mach statt
    perror("slow_syscall");
    exit(EXIT_FAILURE);
    einfach throw "slow_syscall", ich denke bashar hat da nix dagegen, oder?



  • Dr. Zoidberg: Gefällt mir ganz gut.

    Devil81: Sieht mir eher nach Vergewaltigung einer for-Schleife aus. Ausserdem ist das nicht richtig: Wenn restart false ist, wird der Systemaufruf gar nicht erst gemacht, ist es true, wird immer wieder restartet, egal ob EINTR oder nicht. Dazu sind rc und errno bei der ersten Iteration undefiniert.

    Hexagon: Deine Schleife bricht im Normalfall auch nie ab.

    Cengiz: Wenn restart == true, wird deine Schleife nicht betreten.

    Dr Greenthumb: Nicht ganz richtig, wenn restart == false ist, aber der Syscall mit EINTR beendet wird, behandelst du das als fatalen Fehler.

    Bitsy: Die erste Version wär auch mein erster Gedanke gewesen (hast du einen PAP gezeichnet und direkt umgeformt?:)) Hat IMHO 2 mini-Nachteile: Zum einen sehen die Bedingungen seltsam aus ... das "< 0" wird von jedem sofort als Fehler-Überprüfung erkannt, bei ">= 0" muß man erst nachdenken, genauso bei dem "!= EINTR". Zum anderen ist die Bedingung der do..while-Schleife konstant, man könnte das jetzt auch als Vergewaltigung des Sprachmittels ansehen.
    Die zweite Version ist die von Dr. Greenthumb mit Korrektur. Ist OK. Ich kann mich nicht ganz entscheiden zwischen der und der von Dr. Zoidberg ... wobei ich nicht ganz überzeugt bin dass goto tatsächlich unpassend ist. Abgesehen davon dass es evil ist 😉

    Ich hätte gar nicht gedacht, dass es soviele falsche Vorschläge gibt. Ist der Original-Code zu unverständlich -- oder ist fanatische goto-Vermeidung doch nicht so das gelbe vom Ei?



  • Original erstellt von Bashar:
    Ich hätte gar nicht gedacht, dass es soviele falsche Vorschläge gibt. Ist der Original-Code zu unverständlich -- oder ist fanatische goto-Vermeidung doch nicht so das gelbe vom Ei?

    Definitiv unverständlich. Aber die semantik von slow_syscall() ist so unverständlich. Ein zweifach bedeutungsüberladener returnwert!!!

    nu kanns sein, daß in anbetracht einer api mit solchen returnwerten das goto sinnvoll ist. müssen wir wenigstens glauben, solange keine gut lesbare lösung reinflattert.
    hier sollten alle non-goto-dogmaten aufgefordert sein, ihr dogma zu verteidigen. *grins*



  • Original erstellt von volkard:
    Ein zweifach bedeutungsüberladener returnwert!!!

    Nicht wirklich. < 0 ist Fehler, alles andere bedeutet korrekte Abarbeitung.

    Nun gibt es solche und solche Fehlerursachen ... EINTR ist nun gerade ein Fehler, bei dem man es sofort nochmal versuchen kann, es sei denn man mißt dem Signal irgendeine Bedeutung bei, die das verhindert. Bei anderen Fehlern müßte man u.U. etwas Arbeit vor dem neustarten investieren, und bei manchen kann man nur noch aufgeben (Systemweites Semaphor unter dem ***** weggelöscht? Mist.)



  • ich hab gute erfahrungen gemacht, die ganze win32-api zu kapseln
    durch exceptionwerfende inline-funktionen. mal sehen, ob linux
    das auch mag...

    //da in der cpplinux.h
    inline int cpp_slow_syscall()
    {//die dreifache returnwertüberladung auflösen in nur einen 
      //returnwert und zwei exceptions
       int rc=slow_syscall();
       if(rc<0)
          if(errno==EINTR)
             throw Eintr();
          else
             throw Error(errno);
       return rc;
    }
    
    //da wo der das timeout wegignorieren mag
    do
    {
       try
       {
          cpp_slow_syscall();
          return;//break ist lame, und wers braucht, hat diese 
             //funktion zu fett angelegt, füchte ich
       }
       catch(Eintr&)
       {
       }
    }while(again);
    
    //und irgenwo außen meinetwegen
    catch(Error& e)
    {
       perror("slow_syscall");
       exit(EXIT_FAILURE);
    }
    

    ich denke, mit meinem derzeitigen erkenntnisstand würde ich linux so benutzen. ich halts eigentlich für gut lesbar. was meinst du?



  • Also ich würde es so schreiben:

    while(slow_syscall() < 0)
        {
            if(errno != EINTR)
            {
                perror("slow_syscall");
                exit(EXIT_FAILURE);
            }
    
            if(!restart)
                break;
        }
    

Anmelden zum Antworten