13

I have a bit unusual situation - I want to use goto statement to jump into the loop, not to jump out from it.

There are strong reasons to do so - this code must be part of some function which makes some calculations after the first call, returns with request for new data and needs one more call to continue. Function pointers (obvious solution) can't be used because we need interoperability with code which does not support function pointers.

I want to know whether code below is safe, i.e. it will be correctly compiled by all standard-compliant C/C++ compilers (we need both C and C++).

function foo(int not_a_first_call, int *data_to_request, ...other parameters... )
{
    if( not_a_first_call )
        goto request_handler;
    for(i=0; i<n; i++)
    {
        *data_to_request = i;
        return;
request_handler:
        ...process data...
    }
}

I've studied standards, but there isn't much information about such use case. I also wonder whether replacing for by equivalent while will be beneficial from the portability point of view.

Thanks in advance.

UPD: Thanks to all who've commented!

  1. to all commenters :) yes, I understand that I can't jump over initializers of local variables and that I have to save/restore i on each call.

  2. about strong reasons :) This code must implement reverse communication interface. Reverse communication is a coding pattern which tries to avoid using function pointers. Sometimes it have to be used because of legacy code which expects that you will use it.

Unfortunately, r-comm-interface can't be implemented in a nice way. You can't use function pointers and you can't easily split work into several functions.

Clifford
  • 88,407
  • 13
  • 85
  • 165
Sergey
  • 131
  • 1
  • 1
  • 4
  • 9
    "strong reasons to do so" -- I haven't seen them in this post. – Joe May 16 '11 at 18:53
  • 1
    can you not make what is below *request_handler* in a separate function and then just call the function? – Tony The Lion May 16 '11 at 18:54
  • 7
    if you declare variables which are local to the for loop *before* the label, then behavior of expressions using those variables is undefined. Actually, my bet is that any assumption about the state of the stack is moot. You are clearly looking for **coroutines** here, maybe some google with that word can help you. – Alexandre C. May 16 '11 at 18:54
  • For instance [this link](http://www.akira.ruc.dk/~keld/research/COROUTINE/) may be of some use. – Alexandre C. May 16 '11 at 18:56
  • 12
    Obligatory: http://xkcd.com/292/ – Xeo May 16 '11 at 18:59
  • This is not real code. The index variable, `*data_to_request` will always be set to one, unless the index variable is assigned a value elsewhere in the `for` loop. Please edit your post and also show where the index variable is defined. – Thomas Matthews May 16 '11 at 19:14
  • I really hope that `i` is not *really* global (it is not declared). A global and a goto in one go! Brilliant! And a global called `i` to boot! – Clifford May 16 '11 at 19:22
  • http://blog.think-async.com/2009/07/wife-says-i-cant-believe-it-works.html – Fred Nurk May 16 '11 at 19:34
  • Add your question about @Ben Voigt's reply as a comment to that reply not to your question! And yes it is legal in C++ also, but best pretend you'd never seen it. Your other comments should be appended as comments to the relevant replies also or as comments. – Clifford May 17 '11 at 18:22
  • 1
    I know this is old, but just curious on why you don't just call a function (not a function pointer as you said just a regular function to do the same task)? Maybe I am not understanding correctly? – pqsk Oct 17 '13 at 15:53

7 Answers7

13

Seems perfectly legal.

From a draft of the C99 standard http://std.dkuug.dk/JTC1/SC22/WG14/www/docs/n843.htm in the section on the goto statement:

[#3] EXAMPLE 1 It is sometimes convenient to jump  into  the
   middle  of  a  complicated set of statements.  The following
   outline presents one possible approach to a problem based on
   these three assumptions:

     1.  The  general initialization code accesses objects only
         visible to the current function.

     2.  The  general  initialization  code  is  too  large  to
         warrant duplication.

     3.  The  code  to  determine  the next operation is at the
         head of the loop.  (To  allow  it  to  be  reached  by
         continue statements, for example.)

           /* ... */
           goto first_time;
           for (;;) {
                   // determine next operation
                   /* ... */
                   if (need to reinitialize) {
                           // reinitialize-only code
                           /* ... */
                   first_time:
                           // general initialization code
                           /* ... */
                           continue;
                   }
                   // handle other operations
                   /* ... */
           }

Next, we look at the for loop statement:

[#1]  Except for the behavior of a continue statement in the |
   loop body, the statement

           for ( clause-1 ; expr-2 ; expr-3 ) statement

   and the sequence of statements

           {
                   clause-1 ;
                   while ( expr-2 ) {
                           statement
                           expr-3 ;
                   }
           }

Putting the two together with your problem tells you that you are jumping past

i=0;

into the middle of a while loop. You will execute

...process data...

and then

i++;

before flow of control jumps to the test in the while/for loop

i<n;
Jose_X
  • 1,064
  • 8
  • 12
5

Yes, that's legal.

What you're doing is nowhere near as ugly as e.g. Duff's Device, which also is standard-compliant.

As @Alexandre says, don't use goto to skip over variable declarations with non-trivial constructors.


I'm sure you're not expecting local variables to be preserved across calls, since automatic variable lifetime is so fundamental. If you need some state to be preserved, functors (function objects) would be a good choice (in C++). C++0x lambda syntax makes them even easier to build. In C you'll have no choice but to store state into some state block passed in by pointer by the caller.

Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
  • it might be legal, but I still consider a very **strange** thing to do – Tony The Lion May 16 '11 at 19:05
  • Duff's Device doesn't use `goto`. – Sjoerd May 16 '11 at 19:12
  • 3
    Duff's Device uses **switch**, which is a form of goto. To Tony The Tiger: I've updated initial question with some reasons behind such solution. – Sergey May 16 '11 at 19:25
  • 3
    @Sergey, @Sjoerd: The relevant fact is that Duff's Device also jumps into the middle of a loop. It's ugly because of the way the grammar constructs of `switch` (the `case` labels) and the loop are intermingled. That's also the beauty :) – Ben Voigt May 16 '11 at 21:38
1

First, I need to say that you must reconsider doing this some other way. I've rarely seen someone using goto this days if not for error management.

But if you really want to stick with it, there are a few things you'll need to keep in mind:

  • Jumping from outside the loop to the middle won't make your code loop. (check the comments below for more info)

  • Be careful and don't use variables that are set before the label, for instance, referring to *data_to_request. This includes iwhich is set on the for statement and is not initialized when you jump to the label.

Personally, I think in this case I would rather duplicate the code for ...process data... then use goto. And if you pay close attention, you'll notice the return statement inside your for loop, meaning that the code of the label will never get executed unless there's a goto in the code to jump to it.

function foo(int not_a_first_call, int *data_to_request, ...other parameters... )
{
    int i = 0;
    if( not_a_first_call )
    {
        ...process data...
        *data_to_request = i;
        return;
    }

    for (i=0; i<n; i++)
    {
        *data_to_request = i;
        return; 
    }
}
Community
  • 1
  • 1
karlphillip
  • 92,053
  • 36
  • 243
  • 426
  • In this specific case, the user is only initializing **i** in the for loop, which is not executed. Most probably **i** will store garbage which will not satisfy the loop condition. Updated code. – karlphillip May 16 '11 at 19:22
  • 1
    The return doesn't create a problem since the line before the loop appears to redirect traffic past the return in every case. As for why have a for loop that won't execute an additional cycle? Well, it does at least serve to provide a test (i – Jose_X May 16 '11 at 19:32
  • *appears to redirect traffic*. I don't *usually* work on assumptions. The fact is ... the OP wrote a poor piece of code for us to help him. – karlphillip May 16 '11 at 19:41
  • " I've rarely seen someone using goto". Linux kernel, and not just for error management, also for cleanup, and other cases; https://www.kernel.org/doc/html/v4.17/process/coding-style.html rationale for `goto` lists "unconditional statements are easier to understand and follow" and "nesting is reduced". – ShinTakezou Oct 28 '22 at 13:43
0

No, you can't do this. I don't know what this will do exactly, but I do know that as soon as you return, your call stack is unwound and the variable i doesn't exist anymore.

I suggest refactoring. It looks like you're pretty much trying to build an iterator function similar to yield return in C#. Perhaps you could actually write a C++ iterator to do this?

Sean Edwards
  • 2,062
  • 1
  • 18
  • 19
  • 2
    I'm assuming `i` is a parameter, since there's no local definition and it is indicated that there are additional parameters. – Ben Voigt May 16 '11 at 19:02
  • 2
    Yes, I understand that I have to restore value of `i`. In the real code it is passed along with parameters, packed in the special structure and saved-restored on each call. – Sergey May 16 '11 at 19:10
0

It seems to me that you didn't declare i. From the point of declaration completely depends whether or not this is legal what you are doing, but see below for the initialization

  • In C you may declare it before the loop or as loop variable. But if it is declared as loop variable its value will not be initialized when you use it, so this is undefined behavior. And if you declare it before the for the assignment of 0 to it will not be performed.
  • In C++ you can't jump across the constructor of the variable, so you must declare it before the goto.

In both languages you have a more important problem, this is if the value of i is well defined, and if it is initialized if that value makes sense.

Really if there is any way to avoid this, don't do it. Or if this is really, really, performance critical check the assembler if it really does what you want.

Jens Gustedt
  • 76,821
  • 6
  • 102
  • 177
0

The initialisation part of the for loop will not occur, which makes it somewhat redundant. You need to initialise i before the goto.

int i = 0 ;
if( not_a_first_call )
    goto request_handler;
for( ; i<n; i++)
{
    *data_to_request = i;
    return;
request_handler:
    ...process data...
}

However, this is really not a good idea!

The code is flawed in any case, the return statment circumvents the loop. As it stands it is equivalent to:

int i = 0 ;

if( not_a_first_call )
    \\...process_data...

i++ ;
if( i < n )
{
    *data_to_request = i;
}

In the end, if you think you need to do this then your design is flawed, and from the fragment posted your logic also.

Clifford
  • 88,407
  • 13
  • 85
  • 165
  • 1
    No, the "..process_data.." is inside the loop. – Jose_X May 16 '11 at 19:28
  • 1
    After the `... process_data` part comes the loop increment `i++`. – Fred Foo May 16 '11 at 19:37
  • @Jose_X: Yes, it *was* but it need not be to be equivalent. Walk through the code; `...process data...` is executed only when `not_a_first_call` is true, and *before* the for loop statement is processed because of the `goto`. Then the `i++` and `i – Clifford May 17 '11 at 18:13
  • @larsmans: Good spot (edited), but I wonder if it really matters? It depends on whether `i` is truely external to this functuion and the omission of the initialisation was intentional, and similarly on the value of `n`. If `i` and `n` are truly external to the function, this code has more problems than just the abuse of a `goto` and for loop! – Clifford May 17 '11 at 18:13
  • that depends on the value of `n`. Whether it "really matters" depends on whether the OP really has a use for this (pardon my language) crufty piece of spaghetti code. – Fred Foo May 17 '11 at 18:17
0

If I understand correctly, you're trying to do something on the order of:

  • The first time foo is called, it needs to request some data from somewhere else, so it sets up that request and immediately returns;
  • On each subsequent call to foo, it processes the data from the previous request and sets up a new request;
  • This continues until foo has processed all the data.

I don't understand why you need the for loop at all in this case; you're only iterating through the loop once per call (if I understand the use case here). Unless i has been declared static, you lose its value each time through.

Why not define a type to maintain all the state (such as the current value of i) between function calls, and then define an interface around it to set/query whatever parameters you need:

typedef ... FooState;

void foo(FooState *state, ...)
{
  if (FirstCall(state))
  {
    SetRequest(state, 1);
  }
  else if (!Done(state))
  {
    // process data;
    SetRequest(state, GetRequest(state) + 1);
  }
}
John Bode
  • 119,563
  • 19
  • 122
  • 198
  • 1
    The value of i isn't necessarily lost since it is likely declared elsewhere, potentially being a global variable. Additionally, the i – Jose_X May 16 '11 at 19:41