• (cs)

    And, uh, why is it ok to use them here?!

  • (cs) in reply to DelawareBoy
    DelawareBoy:
    And, uh, why is it ok to use them here?!


    That's the point...  It's NOT okay to use them here.
  • (cs) in reply to DelawareBoy

    'K, I'm lost.  Is the implication that the poster should have used
        continue;
    instead of
        goto cont;
    ?

    But continue; re-evaluates the loop condition...

    I suppose something like

    while (true)
    {
        if (sysmgr->getProcessCount() == 0)
        {
           break;
        }

        ...
            continue; // rather than goto cont
        ...
    }

    would work.

  • (cs) in reply to DelawareBoy

    Part of the point is it's not ok.

    Further, part of the irony is that he's using the 'cont' label as the target of his goto. He probably tried 'continue' but found it was a pesky compiler reserved keyword. In most C derived languages, the continue keyword jumps execution back to evaluating the conditional of whatever loop you're currently in, without carrying out the rest of the body of the loop. (He could have used the statement 'continue;' without goto labels to get the same effect.)

  • BrainMaster (unregistered) in reply to DelawareBoy

    I guess that guy skipped the lesson when his class learned "break" and "continue" keywords... It seems it didn't miss the "goto" class, but he should have paid more atention to it!

  • (cs) in reply to DelawareBoy

    A very weird case:  He clearly knows about while loops and he seems to have spent a bit of time thinking about what needs to happen . . . wonder why he failed to implement this better?  Oh yeah!  He must be an idiot.  [:^)]

    The right solution of course is to encapsulate the logic for stopping these processes where they belong -- with the sysmgr (or barring that, in a wrapper for the sysmgr).

  • (cs)

    Well, at least he reinvented the wheel round. I think it is completely equivalent to "continue;". Can someone find a case where it is not equivalent to while and continue?

  • (cs) in reply to DelawareBoy

    C++ implementing GOTO?!?!?

    Now I have seen it all.

  • (cs) in reply to WTFer
    WTFer:
    Well, at least he reinvented the wheel round. I think it is completely equivalent to "continue;". Can someone find a case where it is not equivalent to while and continue?


    The only thing I can think of right off the bat is that using goto cont; will not re-evaluate the while condition.  Is that not the case in this snippet?
  • (cs) in reply to WTFer

    As astutely pointed out earlier, the goto with the continue doesn't re-evaluate the condition of the loop, whereas 'continue;' would. For example, consider get(), if you evaluate it second time, you'll read the next character from standard input. With the goto construct you won't advance your position in the stream, with 'continue;' you do.

  • (cs) in reply to Ytram 
    Ok, is there a reason why this rewrite would be unacceptable?

    int i = 0;
    while    ( (sysmgr->CurrentProcess()->IsActive()) && (i<3) )
    {
      //inactivation is not guaranteed and may take up to 3 calls
      sysmgr->CurrentProcess()->TryInactivate(); //im assuming calling multiple times is OK
      Sleep(DEFAULT_TIMEOUT);
      i++;
    }


    while (sysmgr->getProcessCount() != 0)
    {
      //disconnect child processes
      if( sysmgr->CurrentProcess()->HasChildProcesses() ) //may be superflous, i dunno
      {
        /* ED: Snip */
      }
    }
       
    if( sysmgr->CurrentProcess()->IsReusable() )
    {
      sysmgr->ReuseCurrentProcess();
    }  

    sysmgr->CloseCurrentProcess();

  • (cs) in reply to WTFer
    Can someone find a case where it is not equivalent to while and continue?

    You mean when GOTOs are not the same as loop constructs (while/for)? Well I used it in a parser state machine once to save myself from function calls etc. It may have been a bit spaghetti-ish, but it ran really FAST. =]

  • (cs) in reply to Ytram
    Ytram:
    WTFer:
    Well, at least he reinvented the wheel round. I think it is completely equivalent to "continue;". Can someone find a case where it is not equivalent to while and continue?


    The only thing I can think of right off the bat is that using goto cont; will not re-evaluate the while condition.  Is that not the case in this snippet?


    Thanks, so actually it is a square.. :D
  • Anonymous (unregistered)

    I always use COMEFROM statements instead of GOTO statements.

  • (cs)

    No timeouts when checking to see if the process is inactive. This could be by design, but judging by the comment ("may take up to 3 calls") I'd say it's not by design at all.

    Also, since he's using goto instead of continue, he doesn't check to see if the process count is zero. So if I have one process, and it quits early...

    Programmers this bad need to go back to Javascript. Maybe even XML. You can't do goto in XML.


  • Effendi (unregistered) in reply to WTFer
    Well, at least he reinvented the wheel round. I think it is completely equivalent to "continue;". Can someone find a case where it is not equivalent to while and continue? Well, I think the difference is that the while loop won't be re-evaluated when the code is directed to the goto. That's the only thing I can think of.
  • Pradeep Padala (unregistered)
    Alex Papadimoulis:

    Everybody knows that you should never use "goto" statements. Well, except in one or two rare circumstances that you won't come across anyway. But even when you do come across those situations, they're usually "mirage cases" where there's no need to "goto" anyway.


    Before writing such commentary, think about kernel code where gotos are used extensively. See, http://kerneltrap.org/node/553.

  • (cs)

    Yaknow, it's this little gem that really has me obsessing:

    //inactivation is not guaranteed and may take up to 3 calls
      sysmgr->CurrentProcess()->TryInactivate();
     
      if( sysmgr->CurrentProcess()->IsActive() )
      {
        Sleep(DEFAULT_TIMEOUT);
        goto cont;
      }

    I love the TryInactivate (I'll ignore the use of Inactivate instead of Deactivate).  "Try" indeed, clearly it fails sometimes!  The WTF is that you have a method that may or may not work! Why can't the sysmgr do what it's told to do? Who writes like this?  I mean, if it works then it works, if it doesn't, raise a !#$%!exception.  Don't make me guess!

  • (cs) in reply to christoofar

    "Ok, is there a reason why this rewrite would be unacceptable?"

    Yeah, why not? The rewrite looks very much different. I'm not going to check the logic; I'm a litle drunk, doing Boole's things makes my brain hurt. Moreover, in the original logic TryInactivate() can be called way more than 3 times: WTF? "I know, I'll pull i<3 out of my (*)!" Basically this is a very hairy case to rewrite. I'd find out the original programmer's e-mail address for this one. Also, after we ReuseCurrentProcess(), we're going to TryInactivate() it again. WhyTF this would be necessary, only Godess knows. Just CloseCurrentProcess() is not what the original programmer intended.

    May I point out that using the continue or the break statement would be totally different from restarting the loop without checking the condition? The cont: label is right at the beginning of the loop, admittedly weirdly indented.

    I think that goto's should not be considered harmful. If they were, every while, every if, and every logic of any kind would be harmful; they all get compiled to conditional jmp instructions. For this same reason pointers are not harmful. It's just how computers work. Now higher level languages have nice ways of avoiding goto statements. That's a case of 'using higher-level constructs considered wholosome'.

    I think the best way to rewrite this piece of code is to move most of the loop body inside a function. I wouldn't really say this case of using goto is harmful; it keeps all the logic in one piece of code, not in two. But hten again, I'm a little drunkish. I'll try the rewrite, just for kicks. I'll note in advance that a goto is just a really cheap function call.

    while( sysmgr->getProcessCount() != 0 )
    {
      // Yes, I realize "goto" statements are considered harmful,
      // but this is a case where it is OK to use them
      cont:

      //inactivation is not guaranteed and may take up to 3 calls
      sysmgr->CurrentProcess()->TryInactivate();
     
      if( sysmgr->CurrentProcess()->IsActive() )
      {
        Sleep(DEFAULT_TIMEOUT);
        goto cont;
      }

      /* ED: Snip /

      //disconnect child processes
      if( sysmgr->CurrentProcess()->HasChildProcesses() )
      {
        /     ED: Snip /
      }

      /     ED: Snip /
      
      if( sysmgr->CurrentProcess()->IsReusable() )
      {
        sysmgr->ReuseCurrentProcess();
        goto cont;
      } 

      sysmgr->CloseCurrentProcess();

    }

    could, IMNSHO, be rewritten to:

    while(sysmgr->GetProcessCount() != 0) {
      DoTheImportantInactivatingAndReusing()
    }

    void DoTheImportantInactivatingAndReusing() {
      sysmgr->CurrentProcess()->TryInactivate();
      if(sysmgr->CurrentProcess()->IsActive()) {
        sleep(DEFAULT_TIMEOUT)
        DoTheImportantInactivatingAndReusing();
      }
      /     snip /
      if(someconditionthatdoesntreallyapplybecauseofsnipping) {
        /     snip /
      }
      /     snip */
      if(sysmgr->CurrentProcess()->IsReusable()) {
        sysmgr->ReuseCurrentProcess();
        DoTheImportantInactivatingAndReusing();
      }
      sysmgr->CloseCurrentProcess();
    }

    There! Beautiful! Not harmful! No gotos anywhere. Instead of infinite loops, we'll have infinite recursion! But then again, beer. Hmm, duffs.

  • (cs) in reply to joost

    joost:
    May I point out that using the continue or the break statement would be totally different from restarting the loop without checking the condition? The cont: label is right at the beginning of the loop, admittedly weirdly indented.

    Technically, yes, but not functionally, from what I can tell.  The condition that is checked should not change if the last line of the loop is not called (purely guessing from method names.)  Even if that's not true, it appears that executing the body of the loop when the condition is false is wrong.  So calling continue is different, in that it's better.

  • (cs)

    Ahhh.... GOTO. Where have you been?

    This brings me back to the days of Apple IIe BASIC and TI-Basic on my calculator. I don't think I've ever really needed to use it in any other language. There are enough looping constructs out there that GOTO is 98% of the time unnesessary.

    His comment should have read:

    // Yes, I realize "goto" statements are considered harmful,
    // but I'm too lazy/stupid to think of a better way to do this.

  • Trev (unregistered)
    Alex Papadimoulis:

    Everybody knows that you should never use "goto" statements. Well, except in one or two rare circumstances that you won't come across anyway. But even when you do come across those situations, they're usually "mirage cases" where there's no need to "goto" anyway.


    There are cases where goto is very useful, and actually helps readability.  Non-throwing procedure calls can benefit:

    void MakeDeviceObject()
    {
        int iErr;
        iErr = GetDevice();
        if( iErr == ERROR ) goto l_Err;

        iErr = SetDeviceParameter( "Goblin" );
        if( iErr == ERROR ) goto l_Err;

        ...

        return;

        l_Err:
        DestroyDevice();
        Log( "Error making device object!" );
    }

  • (cs) in reply to Maurits

    OK, here's a goto-less version that does what I think he was trying to do...

    bool docheck = true;

    while (true)
    {
        if (docheck && sysmgr->getProcessCount() == 0)
        {
            break;
        }

        ...

        if (...)
        {
            // rather than goto cont
            docheck = false; // skip the check
            continue;
        }
        ...

        docheck = true; // made it to the end... repeat the check
    }

  • (cs) in reply to christoofar

    christoofar:
    Ok, is there a reason why this rewrite would be unacceptable?
    Well, yeah.... "may take up to 3 calls" is techy speak for "it doesn't always work on the first call, but it usually works the 2nd or 3rd time.  And whatevers preventing it from working on the first or second call may (probably will) get worse in the furture.  So, no artifical limits on the number of times we retry (until we can firmly establish the if it hasn't worked after N tries, so other condition if prevent it -- of course, we probably should check for that condition first).

    Secondly, the "timeout" seems to be a "retry delay" -- we only want to pause for that time if the Deactivate failed.

    Also, I'm gonna guess that "Current Process" is just one of getProcessCount() ... When one "Current Process" closes, another one becomes the new "Current Process".

    I believe what we actually want here is :

    while( sysmgr->getProcessCount() != 0 )
{
  sysmgr->CurrentProcess()->TryInactivate();

    //inactivation is not guaranteed and may take up to 3 calls
if( sysmgr->CurrentProcess()->IsActive() )
{
// Wait a bit, then try closing it again.
Sleep(DEFAULT_TIMEOUT);
}
else
{
/* ED: Snip */
    

    //disconnect child processes
if( sysmgr-&gt;CurrentProcess()-&gt;HasChildProcesses() )
{
    /* ED: Snip */
}

/* ED: Snip */
   
if( sysmgr-&gt;CurrentProcess()-&gt;IsReusable() )
{
    sysmgr-&gt;ReuseCurrentProcess();
}  
else
{
    sysmgr-&gt;CloseCurrentProcess();
}

    

    }
}

    No gotos, no continues, no breaks. One comment added, and one moved, so things make more sense.

  • (cs) in reply to joost

    joost:
    "Ok, is there a reason why this rewrite would be unacceptable?"

    Yeah, why not? The rewrite looks very much different. I'm not going to check the logic; I'm a litle drunk, doing Boole's things makes my brain hurt.

    *snip*

    Friends don't let friends code drunk.  They end up on the Daily WTF.

  • (cs) in reply to Maurits

    Maurits:
    OK, here's a goto-less version that does what I think he was trying to do...
    while (true)

    Nope... You haven't eliminated the GOTO, you just renamed it.

    All loops end eventually, on some condition.  Get that condition into your while loop statement.

     

     

  • (cs) in reply to JamesCurran
    JamesCurran:

    Maurits:
    OK, here's a goto-less version that does what I think he was trying to do...
    while (true)

    Nope... You haven't eliminated the GOTO, you just renamed it.

    All loops end eventually, on some condition.  Get that condition into your while loop statement.

     


    Picky, picky...
    bool docheck = true;

    while (docheck && sysmgr->getProcessCount() == 0)
    {
        ...
        if (...)
        {
            // rather than goto cont
            docheck = false;
            continue;
        }
        ...
        docheck = true; // made it to the end, reset
    }

  • (cs)

    This is a first: I don’t consider the code a WTF at all. It could have been written better, sure, but without knowing if re-evaluating the condition is undesirable and how long the snipped code was, it does not warrant a WTF.

    As for GOTO, I have this to say:

    The apprentice uses it without thinking.
    The journeyman avoids it without thinking.
    The master uses it thoughtfully.

  • (cs) in reply to Trev
    Trev:

    There are cases where goto is very useful, and actually helps readability.  Non-throwing procedure calls can benefit:

    void MakeDeviceObject()
    {
        int iErr;
        iErr = GetDevice();
        if( iErr == ERROR ) goto l_Err;

        iErr = SetDeviceParameter( "Goblin" );
        if( iErr == ERROR ) goto l_Err;

        ...

        return;

        l_Err:
        DestroyDevice();
        Log( "Error making device object!" );
    }


    How is that any better than this:

    void MakeDeviceObject()
    {
        try
        {
            GetDevice();
            SetDeviceParameter( "Goblin" );
             ...
        }
        catch(DeviceException)
        {
             DestroyDevice();
            Log( "Error making device object!" );
        }
    }

  • (cs) in reply to Ytram

    Ytram:
    Trev:

    There are cases where goto is very useful, and actually helps readability.  Non-throwing procedure calls can benefit:



    How is that any better than this:

    void MakeDeviceObject()
    {
        try
        { [snip ]

    Well, since, as Trev stated, GetDevice() & SetDeviceParameter() do not throw exceptions, the code in your catch will never be executed (but SetDeviceParameter() will probably fail spectacularly when GetDevice fails gracefully)

  • (cs) in reply to christoofar

    christoofar:
    C++ implementing GOTO?!?!?

    Now I have seen it all.

    Actually...

    http://msdn.microsoft.com/library/default.asp?url=/library/en-us/csref/html/vclrfthegotostatement.asp

    Now you've seen it all.

     

  • (cs) in reply to Maurits

    v3: use a for loop instead

    for (
        bool docheck = true;
        docheck && sysmgr->getProcessCount() != 0;
        docheck = true;
    )
    {
        ...
        if (...)
        {
            // rather than goto cont
            docheck = false;
            continue;
        }
        ...
    }

  • (cs)

    Ok, this is kind of freaky that this showed up.  Last week, a coworker and myself were discussing the C# switch statement and the use of break inside of them.  I mentioned that a great WTF would be if we came across something like this (forgive any syntactical errors for now)

    Switch (myVar)
    {
       case 1:
          doSomething();
          goto endswitch;

       case 2:
          doSomethingElse();
          goto endswitch;

       case 3:
          doSomethingDifferent();
          goto endswitch;
    }

    endswitch:

    I know it's a bit different and this is not code that we actually came across, but since we were just talking about the use of goto's and now it shows up in WTF is a bit spooooooky.  Like they were reading our minds.

  • (cs) in reply to richleick

    this site always makes me feel good about myself.

  • (cs) in reply to Maurits

    Oops v3.1
    for (
        bool docheck = true;
        docheck && sysmgr->getProcessCount() != 0;
        // DON'T docheck = true here as "continue" will fire it
    )
    {
        ...
        if (...)
        {
            // rather than goto cont
            docheck = false;
            continue;
        }
        ...
        // OK to put it here though
        docheck = true;
    }

  • (cs) in reply to Aristotle Pagaltzis
    Aristotle Pagaltzis:

    This is a first: I don’t consider the code a WTF at all. It could have been written better, sure, but without knowing if re-evaluating the condition is undesirable and how long the snipped code was, it does not warrant a WTF.

    I have to agree with this. This might be a "Huh?" but not a "WTF?"

  • (cs) in reply to JamesCurran
    JamesCurran:

    Ytram:
    Trev:

    There are cases where goto is very useful, and actually helps readability.  Non-throwing procedure calls can benefit:



    How is that any better than this:

    void MakeDeviceObject()
    {
        try
        { [snip ]

    Well, since, as Trev stated, GetDevice() & SetDeviceParameter() do not throw exceptions, the code in your catch will never be executed (but SetDeviceParameter() will probably fail spectacularly when GetDevice fails gracefully)

    You could always throw you own exception.  Generally, I would just create a new method, though.

  • (cs) in reply to sdf

    The thing about the 'gotos are sometimes necessary' argument is that Java doesn't allow gotos yet I've never heard of a single one of the thousands of Java projects failing for the want of a goto.

    So until someone can show me a code snippet that uses gotos and cannot be rewritten not to use them, I have to believe that gotos are never necessary.  I've not used a single one since I learned to program in basic.  I've never even desired to use them.  I don't even use labelled breaks.  Never wanted to.

  • (cs)

    Well, I was bored at work, so I figured I'd give it another go:

     

    while( sysmgr->getProcessCount() != 0 )
{
    do 
    {
        sysmgr->CurrentProcess()->TryInactivate();
    } while(sysmgr->CurrentProcess()->IsActive() && (Sleep(DEFAULT_TIMEOUT), true));

    /* ED: Snip */

//disconnect child processes
if( sysmgr-&gt;CurrentProcess()-&gt;HasChildProcesses() )
{
    /* ED: Snip */
}

/* ED: Snip */
   
if( sysmgr-&gt;CurrentProcess()-&gt;IsReusable() )
{
    sysmgr-&gt;ReuseCurrentProcess();
}  
else
{
    sysmgr-&gt;CloseCurrentProcess();
}

    

    }

    If you're having trouble following the line

    <FONT face="Courier New">while(sysmgr->CurrentProcess()->IsActive() && (Sleep(DEFAULT_TIMEOUT), true));</FONT>

    don't feel bad.  I'd probably consider it too WTF-ish to actually use in production code. Here's the translation for the "Obfucated C" impared:

    <FONT face="Courier New">sysmgr->CurrentProcess()->IsActive()</FONT>  is the condition we want to check. Since it's joined to the rest by an AND condition, if it's false, the whole expression must be false, and we exit the loop without evaluating the rest of it..

    <FONT face="Courier New">Sleep(DEFAULT_TIMEOUT), true</FONT>  The comma operator means "evalutate both parts, then use the expression on the right, as the value of the whole expression.  In other words, first we sleep for a bit, then we declare the sub-expression to be true.  Since both halves the expression were true, we continue looping.  Now, if Sleep could be counted on to reliably return a !=0 value, we could make while clause shorter and even more confusing!  (On the other hand, however, I believe if Sleep returns void, I think we're just screwed, and it won't even compile)

  • (cs) in reply to Aristotle Pagaltzis
    Aristotle Pagaltzis:

    As for GOTO, I have this to say:

    The apprentice uses it without thinking.
    The journeyman avoids it without thinking.
    The master uses it thoughtfully.

    Well put!

  • (cs) in reply to dubwai

    dubwai:
    The thing about the 'gotos are sometimes necessary' argument is that Java doesn't allow gotos yet I've never heard of a single one of the thousands of Java projects failing for the want of a goto.

    Actually, going back even further, dBase III didn't have a GOTO either.  (well, it's actually did has a keyword named "goto", but it's not what you think ... it moved the active record in the database to a different row).  And there's been a lot more code written in dbase III then in Java.

    So until someone can show me a code snippet that uses gotos and cannot be rewritten not to use them, I have to believe that gotos are never necessary.  I've not used a single one since I learned to program in basic.  I've never even desired to use them.  I don't even use labelled breaks.  Never wanted to.

    The problem is not if it can or cannot be rewritten without them, but can it be rewritten better by removing them.

     

  • Trev (unregistered) in reply to dubwai
    dubwai:

    The thing about the 'gotos are sometimes necessary' argument is that Java doesn't allow gotos yet I've never heard of a single one of the thousands of Java projects failing for the want of a goto.

    So until someone can show me a code snippet that uses gotos and cannot be rewritten not to use them, I have to believe that gotos are never necessary.  I've not used a single one since I learned to program in basic.  I've never even desired to use them.  I don't even use labelled breaks.  Never wanted to.



    Libraries in Java throw exceptions when exceptional cases happen.  C++ doesn't have that advantage; any library that's designed to be compiled in either C or C++ (many) will not throw exceptions (unless it's using SEH, which is not portable). So, you look at return values.  The example I posted above is used a lot in Win32 and DirectX programming where lots of calls in a row can fail for some reason, and you just want to back out.

    "goto is evil" is pure dogma.  Any feature is only as bad as the worse use of it.  If used well, it's perfectly fine.
  • (cs) in reply to A Wizard A True Star
    A Wizard A True Star:

    Actually...

    Perl stole this idea from FORTRAN:

    goto ("FOO", "BAR", "GLARCH")[$i];

    http://perldoc.perl.org/functions/goto.html

    Use that in production code and die().

  • (cs) in reply to JamesCurran
    JamesCurran:

    So until someone can show me a code snippet that uses gotos and cannot be rewritten not to use them, I have to believe that gotos are never necessary.  I've not used a single one since I learned to program in basic.  I've never even desired to use them.  I don't even use labelled breaks.  Never wanted to.

    The problem is not if it can or cannot be rewritten without them, but can it be rewritten better by removing them.

    I feel pretty confident that given a piece of code that uses gotos, I can structure the code in a way that doesn't use them and is at least 'as good'.

    Every example I've seen to this point have been straw man examples.  Examples of how gotos make bad code marginally better.  They are too long or do too much or should be replaced with a different approach entirely orientation..  I could be wrong, but gotos seem to come from habit or a lack of imagination i.e. being locked into a procedural mindset.

  • (cs) in reply to Ytram
    Ytram:
    How is that any better than this:

    void MakeDeviceObject()
    {
        try
        {
            GetDevice();
            SetDeviceParameter( "Goblin" );
             ...
        }
        catch(DeviceException)
        {
             DestroyDevice();
            Log( "Error making device object!" );
        }
    }

    Well, for one thing, your code is illegal in C… in languages without exceptions, GOTO can be the only maintainable and readable way of cleaning up allocated resources.

  • (cs) in reply to Trev

    Anonymous:


    Libraries in Java throw exceptions when exceptional cases happen.  C++ doesn't have that advantage; any library that's designed to be compiled in either C or C++ (many) will not throw exceptions (unless it's using SEH, which is not portable). So, you look at return values.  The example I posted above is used a lot in Win32 and DirectX programming where lots of calls in a row can fail for some reason, and you just want to back out.

    If you do not have exceptions, I see using gotos for that purpose as perfectly acceptable.


    "goto is evil" is pure dogma.  Any feature is only as bad as the worse use of it.  If used well, it's perfectly fine.

    I don't see gotos as evil.  I don't worship at the altar of structured programming.  I've programmed microcontrollers and in machine language and understand that basically anything that is useful is a jump.  There are lots of perfectly accpetable uses of gotos.  My thing is that 'thouroughly modern' languages have formalized all of the really useful and acceptable uses.  So if you don't have all the modern tools at your disposal (notably exceptions) then you have to make do.

    Goto is a swiss army knife.  The problem is that when you are making sushi with a swiss army knife the blad my close on your hand and then you bleed all over the sushi.  Then your dinner is ruined and you have to order a pizza and the pizza guy gets mugged because you live in a bad neighorhood and he shoots some poor kid who had a toy gun and then the kids mom gets really sad.  Do you want to make the mothers of poor kids sad?  Do you?!

  • (cs) in reply to dubwai
    dubwai:
    Every example I've seen to this point have been straw man examples. Examples of how gotos make bad code marginally better. They are too long or do too much or should be replaced with a different approach entirely orientation.. I could be wrong, but gotos seem to come from habit or a lack of imagination i.e. being locked into a procedural mindset.

    Well, I certainly prefer this:

    int foo() {
    void * res1, * res2, * res3, * res4, * res5;
    int result = -1;

    res1 = alloc_res1();
    if( ! res1 ) goto leave;

    res2 = alloc_res2( res1 );
    if( ! res2 ) goto dealloc_res1;

    res3 = alloc_res2( res2 );
    if( ! res3 ) goto dealloc_res2;

    res4 = alloc_res2( res3 );
    if( ! res4 ) goto dealloc_res3;

    res5 = alloc_res2( res4 );
    if( ! res5 ) goto dealloc_res4;

    do_something( res5 );

    result = 0

    dealloc_res( res5 );

    delloc_res4:
    dealloc_res( res4 );

    delloc_res3:
    dealloc_res( res3 );

    delloc_res2:
    dealloc_res( res2 );

    delloc_res1:
    dealloc_res( res1 );

    leave:
    return result;
}

    over this:

    int foo() {
    void * res1;
    int result = -1;

    res1 = alloc_res1();
    if( res1 ) {
        void * res2 = alloc_res2( res1 );

        if( res2 ) { 
            void * res3 = alloc_res2( res2 );

            if( res3 ) {
                res4 = alloc_res2( res3 );

                if( res4 ) {
                    res5 = alloc_res2( res4 );

                    if( res5 ) {
                        do_something( res5 );
                        result = 0;
                        dealloc_res( res5 );
                    }

                    dealloc_res( res4 );
                }

                dealloc_res( res3 );
            }

            dealloc_res( res2 );
        }

        dealloc_res( res1 );
    }

    return result;
}

    Procedural mindset? What else would you use in C? Sure, I’ve never needed a GOTO in Perl either.

  • (cs) in reply to Aristotle Pagaltzis
    Aristotle Pagaltzis:
    Well, for one thing, your code is illegal in C… in languages without exceptions, GOTO can be the only maintainable and readable way of cleaning up allocated resources.

    I think the best illustration of this point was made by Robert Love (kerneltrap)


    From: Robert Love
    Subject: Re: any chance of 2.6.0-test*?
    Date: 12 Jan 2003 17:58:06 -0500

    On Sun, 2003-01-12 at 17:22, Rob Wilkens wrote:

    > I say "please don't use goto" and instead have a "cleanup_lock" function
    > and add that before all the return statements.. It should not be a
    > burden. Yes, it's asking the developer to work a little harder, but the
    > end result is better code.

    No, it is gross and it bloats the kernel. It inlines a bunch of junk
    for N error paths, as opposed to having the exit code once at the end.
    Cache footprint is key and you just killed it.

    Nor is it easier to read.

    As a final argument, it does not let us cleanly do the usual stack-esque
    wind and unwind, i.e.

        do A
        if (error)
            goto out_a;
        do B
        if (error)
            goto out_b;
        do C
        if (error)
            goto out_c;
        goto out;
        out_c:
        undo C
        out_b:
        undo B:
        out_a:
        undo A
        out:
        return ret;

    Now stop this.

        Robert Love



    In case it was not clear, say you have three tasks to do, and if you fail any of them, you want to roll your changes back.  The above code is a simple way to do this without a lot of indentation, and it's efficient from a CPU point of view.

  • cratermoon (unregistered) in reply to El Duderino

    Although modern C++ has exceptions, there are valid reasons why a particular shop might choose to eschew them. On the other hand, straight C (which this could very well be) does not have exceptions.

  • (cs) in reply to dubwai
    dubwai:
    If you do not have exceptions, I see using gotos for that purpose as perfectly acceptable.

    Okay, I suppose there is no actual argument then.

Leave a comment on “When It's OK To GOTO”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article