goto statement usage in C++

Question

I have a funcA which I call every msec. by another funcB. I want to use goto statement. But when I look at the flow (when m_tempdata is not NULL), after printing "stage 2", it is also printing "cleanup starts". Normally, I expect to return after printing "stage 2" for the next turn. Am I wrong?

void ClassA::funcA()
{
    m_tempdata = m_freedata;

    printf("stage 1 \n");

    if (NULL == m_tempdata)
    {
        printf("going cleaning \n" );
        goto cleanup;
    }
    m_freedata = m_tempdata->next;

    printf("stage 2 \n");

cleanup: printf("cleanup starts \n");
    // ... some additional work todo
}

Why would you expect it to return? cleanup: is just a label between your statements. Also, there's no good reason to justify goto here :v — user123
– user123, Commented Jan 17, 2014 at 15:15

John Dibling · Accepted Answer · 2014-01-17 15:51:46Z

3

Normally, I expect to return after printing "stage 2" for the next turn. Am I wrong?

Yes, you're wrong.

In the case where the label wasn't jumped to, the code will simply fall through to it and continue. A label isn't an instruction. It doesn't cause the program to magically return or jump somewhere else.

Here is a much better way to write this:

void ClassA::funcA()
{
  m_tempdata = m_freedata;

  printf("stage 1 \n");

  if (NULL == m_tempdata)
  {
    printf("going cleaning \n" );
    Cleanup();
    return;
  }

  m_freedata = m_tempdata->next;
}

void ClassA::Cleanup()
{
  printf("cleanup starts \n");
  //      ... some additional work todo
}

Please don't use goto. It make the code much more difficult to understand, debug and maintain. Plus you end up making silly mistakes like you did here because you made poor design decisions earlier. Use modern flow-control constructs instead.

edited Jan 17, 2014 at 15:51

answered Jan 17, 2014 at 15:33

John Dibling

102k34 gold badges192 silver badges335 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

TypeIA · Accepted Answer · 2014-01-17 15:17:29Z

1

You don't seem to have a good/compelling reason to use goto in this code, so don't. Use more readable control flow structures.

printf("stage 1\n");
if (NULL != m_tempdata)
{
    printf("stage 2\n");
    // ...
}
else
{
    printf("cleanup starts\n");
    // ...
}

If you need to do the cleanup regardless of whether "stage 2" runs or not, remove the else block and have the cleanup code at the end.

answered Jan 17, 2014 at 15:17

TypeIA

17.4k1 gold badge41 silver badges58 bronze badges

Comments

Angew is no longer proud of SO · Accepted Answer · 2014-01-17 15:15:50Z

0

Why should it stop? There's nothing to interrupt control flow between the statement printf("stage 2 \n"); and the statement printf("cleanup starts \n"); (which happens to be labelled cleanup:).

If you really want it to interrupt, you'll have to insert a return statement before the cleanup: label.

But please, seriously reconsider using goto. It makes programs very difficult to reason about - always prefer structured flow (ifs, loops, function calls).

answered Jan 17, 2014 at 15:15

Angew is no longer proud of SO

172k18 gold badges377 silver badges478 bronze badges

Comments

Some programmer dude · Accepted Answer · 2014-01-17 15:16:21Z

0

There's nothing to tell the compiler that the function should exit after "stage 2". If you want it to return you have to actually tell the compiler that by adding a return statement.

answered Jan 17, 2014 at 15:16

Some programmer dude

411k36 gold badges420 silver badges655 bronze badges

2 Comments

Avb Avb Over a year ago

Actually, since cleanup is called by goto statement, I was thinking it is only executed when goto is called. Is it possible to let it work without return?

Some programmer dude Over a year ago

@AvbAvb You can still use return with no expression: return;. It will return from the function (with or without a return value) at the point of the return statement.

kuroi neko · Accepted Answer · 2014-01-17 15:24:04Z

0

I suspect you are trying to use a vintage C programming pattern like this one:

void do_hot_stuff (void)
{
    void * data1 = malloc (123);
    if (data1 == NULL) goto fail1;

    void * data2 = malloc (123);
    if (data2 == NULL) goto fail2;

    void * data3 = malloc (123);
    if (data3 == NULL) goto fail3;

    // do your hot stuff here

    free (data3);
    fail3:
    free(data2);
    fail2:
    free(data1);
    fail1:
}

Frankly I did not use this since I last wrote code for my Amiga back in the late 80's.
Just don't.

answered Jan 17, 2014 at 15:24

kuroi neko

8,6681 gold badge23 silver badges45 bronze badges

3 Comments

Mihai Todor Over a year ago

I think you mixed up the labels and the statements at the end :) You can probably still find this kind of stuff in libraries, such as OpenSSL, but I sure hope that people don't continue to write such code. A few years ago, I saw some production code where most functions contained a giant while loop which wrapped the "hot stuff" in order to catch exceptional cases. Nasty stuff, but still better than goto :)

kuroi neko Over a year ago

@MihaiTodor Mmmm... I think the statement placement is OK (if you fail to allocate data3, you just have to free data2 & data1), but anyway this stuff is good for museums.

Mihai Todor Over a year ago

Ah, right :)) Another good example to never use such things :)

Collectives™ on Stack Overflow

goto statement usage in C++

5 Answers 5

Comments

Comments

Comments

2 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

Comments

Comments

2 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related