Sign in to follow this  
Gage64

The use of goto in C (not C++)

Recommended Posts

Say I have this function:
void func() {
    int *arr = (int*)malloc(sizeof(int) * 50);
    if (!arr) {
        printf("Error allocating memory");
freeMem:
        free(arr);
        return;
    }
    
    FILE *file = fopen("myfile.txt", "t");
    if (!file) {
        printf("Failed to open file");
        goto freeMem;
    }
    
    if (some error) {
closeFile:
        fclose(file);
        goto freeMem;
    }
    
    if (some error)
        goto closeFile;
    
    if (some error)
        goto closeFile;
    
    /* ... */
}


I'm not used to programming in straight C, so the code may contain some errors. Do you think the use of goto is justified here? If not, how would you rewrite the code to avoid it? Although it's already stated in the title, I'll say it again - assume that you have to write the function is straight C. You can't use any C++ features. [Edited by - Gage64 on July 3, 2008 11:07:42 AM]

Share this post


Link to post
Share on other sites
I think goto is OK here, but I think you can simplify the logic just a little bit. Something like this would be better:

void func( )
{
int *arr = (int*)malloc(sizeof(int) * 50);
FILE *file = fopen("myfile.txt", "t");

if(!arr || !file)
goto end;

if (some error)
goto end;

if (some error)
goto end;

if (some error)
goto end;

/* ... */

end:
if(arr) free(arr);
if(file) close(file);
}


That is the cleanest thing I can think of without seeing more code. It looks like this function is trying to do too much though. Can you split this up at all?

Share this post


Link to post
Share on other sites
Quote:
Original post by Simian Man
That is the cleanest thing I can think of without seeing more code. It looks like this function is trying to do too much though. Can you split this up at all?


It was just a theoretical question, I don't actually have to write something like this (thankfully [smile]).

And yes, your solution is much nicer, although I think the first if statement is not needed Never mind, I'm an idiot [smile].

Share this post


Link to post
Share on other sites
I do consider this an idiom, so I wouldn't worry about it too much if I encountered it in code. However, I would probably write things as such myself, to further encapsulate the various allocations:


RESULT func()
{
int *buf = malloc(sizeof(int) * 50);
RESULT r;

if (!buf) return ERROR_ALLOC;

r = read_file(buf);

free (arr);
return r;
}

RESULT read_file(int *target)
{
FILE *file = fopen("myfile.txt", "r");
RESULT r;

if (!file) return ERROR_FILE_OPEN;

r = do_funny_things(target, file);

fclose(file);
return r;
}

Share this post


Link to post
Share on other sites
Quote:
Original post by Gage64
And yes, your solution is much nicer, although I think the first if statement is not needed.


Yeah you're right. I knew delete was safe on NULL pointers, but I couldn't remember about free (it is safe).

Share this post


Link to post
Share on other sites
I realize it's not a real world example, but I would assume we only need to allocate memory if we are actually able to open the file.

You could choose to check the pointers at the end of the function:

FILE *file = 0;
int *arr = 0;

if (some error)
/* stuff to handle this specific error */
else if (some error)
/* stuff to handle this specific error */
else if (some error)
/* stuff to handle this specific error */

/* more code.. */

if (file)
fclose(file)

Share this post


Link to post
Share on other sites
Quote:
Original post by Simian Man
Quote:
Original post by Gage64
And yes, your solution is much nicer, although I think the first if statement is not needed.


Yeah you're right. I knew delete was safe on NULL pointers, but I couldn't remember about free (it is safe).


I corrected my post. It is definitely needed because if either one is NULL, the function should obviously not continue executing. BTW, I meant the if statement at the top of the function.

Share this post


Link to post
Share on other sites
Quote:
Original post by WanMaster
You could choose to check the pointers at the end of the function:


I'm not sure what you mean. If you failed to open the file, you have to know about it right away, so how can you check for this only at the end of the function?

Share this post


Link to post
Share on other sites
Quote:
Original post by Gage64
Quote:
Original post by WanMaster
You could choose to check the pointers at the end of the function:


I'm not sure what you mean. If you failed to open the file, you have to know about it right away, so how can you check for this only at the end of the function?

FILE *file = 0;
int *arr = 0;

if (some error) {
display_error("file not found"); }
else if (some error) {
display_error("file in use"); }
else if (some error) {
display_error("permission denied"); }
else {
/* no error, do stuff with file */
arr = (int*)malloc(sizeof(int) * 50);
/* etc. */
}

if (file)
fclose(file) /* of course in this simple case this could be done within the else-scope */




Well, what I meant to demonstrate is having a single point in code where the file is closed (or memory is released) without the need of a label and its jump. Admittedly, as the function body grows in complexity this will become very tricky, but in that case it should have be split up in to multiple procedures anyway.

Share this post


Link to post
Share on other sites
I'd prefer putting the upper part in a seperate function, however, a common way to avoid goto is abusing a switch or loop.


do {
if (bla)
break;

if (foo)
break;
} while(0);

//cleanup

of

switch(whatever)
{
default:
if (bla)
break;
...
}




Frankly, I'd rather have a clear and simple "goto cleanup" than this form of raping unrelated constructs.. unless you could put the code in question in its own function and just return in different places, which I'd consider the "cleanest" solution.

Share this post


Link to post
Share on other sites
(As long as we're talking C, here) I think a goto here or there can be of great help. But I wouldn't use more than one inside a single function -- that's when people start having to put in additional effort to understand what's going on. Your ultimate goal is to avoid that unneeded effort.

The whole point of using a goto (I assume) is for brevity because brevity often aids clarity. Too much brevity may have the opposite effect to that which is desired.

EDIT: by "one", I mean one *label*, not necessarily a single goto statement.

Share this post


Link to post
Share on other sites
I spent a lot of years programming in FORTRAN IV, where GOTO was pretty much the only control flow mechanism (although, to be fair, there were two different kinds of GOTO, so there was a certain kind of richness to the language). I learned through maintaining a lot of other people's code that an excellent rule is to only ever jump forward. Jumping backwards makes spaghetti.

In C there is never a good reason to use goto: wise use of functions will almost always do a better job of making maintenance easier. That said, if you insist on writing idiomatic FORTRAN in C, make sure all your jumps go forwards.

Share this post


Link to post
Share on other sites
Quote:
Original post by thedustbustr
Linux: Using goto In Kernel Code (traditional fun Linus rant).

Linux Device Drivers, 2nd Edition; Chapter 2: Building and Running Modules; Error Handling in init_module

Interesting read. Thanks for sharing.

I've always been a big "ZOMG I FUCKING HATE GOTOS, I WILL MURDER YOU IF YOU USE THEM" type of individual. However, I suppose that like all the language features, if abused and over used incorrectly, it results in terrible code. Those examples as presented in the quoted link really do seem to function better as gotos.

I definitely won't be in any hurry to use goto in my code though...
kernel coding isn't quite what I'm doing...

Share this post


Link to post
Share on other sites
@Gage64:
void func() {
int some_error = 0;

int *arr = (int*)malloc(sizeof(int) * 50);
if (!arr) {
printf("Error allocating memory");
some_error = 1;
}
FILE *file = fopen("myfile.txt", "t");
if (!file) {
printf("Failed to open file");
some_error = 1;
}

if (! some_error) {
/* regular operation */
} else {
/* cleanup */
if(file) {
fclose(file);
}
if(arr) {
free(arr);
}
}

The code now reads from top to bottom and requires no backtracking to understand.

Share this post


Link to post
Share on other sites
I love gotos, i just never have a chance to use it :( (thats good tho)

usually i do something like

{
long ret;
FILE *f, *f2;
f = fopen(...);
if (f == 0)
{
ret = -1;
goto errHdl;
}
f2 = fopen(...);
if (! f2) //90% of times i do == but the odd time i like ! then a space)
{
ret = -2;
goto errHdl;
}
...
ret = 0;
errHdl:
if (f2)
fclose(f2);
if (f)
fclose(f);
return ret;
}



The other time i use goto is

while (loopCond1)
{
while (loopCond2)
{
if (x == OMG)
{
val = something;
goto breakOut;
}
}
}
breakout:
some processing here related to loop above
//then code for rest of the func, if applies



also, you may like alloca(); You dont need to free() it. The memory is on stack so its the equivalent of func(size_t size) { char arr[size]; } except sometimes your code is more complex. ex

long func(_t someStruct)
{
char *table[someStruct.length];

for (size_t i=0; i<someStruct.length; i++)
{
if (someStruct.arr[i].val != 0)
{
table[i] = alloca(someStruct.arr[i].len);
memcpy(table[i], someStruct.arr[i].val, someStruct.arr[i].len);
}
}
//process table
}


I actually had to do something like that but much more complex. I was taking strings, arrays and ints and converting it to text which i then process with a tableToSql() function. Yikes! alloca was really nice to use there. Except in my case my tables were small enough to fit on the stack. At least for a while, i then later had added support for bin files and i easily replaced the alloca with malloc and add a delete at the end of the func. So the advantage to this is how easy you can convert it to malloc instead of doing some hack with a bunch of { char arr[XSize]; }

Share this post


Link to post
Share on other sites
Sometimes its hard to deal with these cases where you have to release multiple resources.

Now you may use "C"'s ability to create dynamic arrays on stack:
I'm assuming you're doing this:
int *arr = (int*)malloc(sizeof(int) * 50);
because in reality '50' is a variable 'x', in which case you can do:
int arr[x];
C++ doesn't handle this, so you may not be able to compile it if you're using C++ compiler, or old C compiler.
Here's a full program that compiles:

#include <stdio.h>
#include <stdlib.h>

int
main(int argc, char **argv)
{
int arr[argc > 1 ? atoi(argv[1]) : 5];

printf("sizeof(arr) == %d\n", sizeof(arr));
return 0;
}



Another 'trick', though I'm not in love with it, is:


void func() {
FILE *file = NULL;
int *arr = (int*)malloc(sizeof(int) * 50);
int got_error = 1;
if (!arr) {
printf("Error allocating memory");
// you shouldn't free NULL, and // is a valid C comment now
return;
}

while (1)
{
file = fopen("myfile.txt", "r");
if (!file) {
printf("Failed to open file");
break;
}

if (some error) {
break;
}

if (some error)
break;

if (some error)
break;

/* ... */
got_error = 0;
break;
}

free(mem);
if (file) fclose(file);
if (got_error) fprintf(stderr, "I got error!!!!\n");
return 0;
}



(code is not indented because 'tab' doesn't seem to be working in this window :(

Share this post


Link to post
Share on other sites
Quote:
Original post by Oluseyi
@Gage64:
...
The code now reads from top to bottom and requires no backtracking to understand.


Like I said, my example was illustrative. You are assuming that you can perform further operations (attempt to open a file) even if previous operations failed (the memory allocation). In your code, that's true, but what if one operation depends on a previous operation being successful? If the previous operation failed, you cant continue - you have to exit the function immediately, while making sure to cleanup any allocated resources.

Still, it does show that elegantly avoiding a goto is not as hard as I thought in this particular case.

I'm not sure if ToohrVyk's approach suffers from the same problems or not. I wish I had a complicated real world example to test it with...

Share this post


Link to post
Share on other sites
Quote:
Original post by Gage64
Do you think the use of goto is justified here?


Not really. Lots of C programmers would use goto to handle this kind of thing, but they'd also generally structure it rather differently. :)

Quote:
If not, how would you rewrite the code to avoid it?



/* NOTE: ONLY func() gets mentioned in the header file. Unless you have a good reason otherwise. :) */

void func_mem_file(int* arr, FILE* file) {
if (some error) { return; }
/* ... */
if (some error) { return; }
/* ... */
if (some error) { return; }
/* ... */
}

void func_mem(int* arr) {
FILE *file = fopen("myfile.txt", "t");
if (file) { func_mem_file(arr, file); }
else { printf("Failed to open file"); }
fclose(file); /* this could instead go in the if-clause above. */
}

void func() {
int *arr = (int*)malloc(sizeof(int) * 50);
if (arr) { func_mem(arr); }
else { printf("Error allocating memory"); }
free(arr); /* this could instead go in the if-clause above. */
}






Quote:
Original post by AcidZombie24
I love gotos, i just never have a chance to use it :( (thats good tho)
...
The other time i use goto is


Wait, what?

@abeylin: free()ing NULL is fine, just like delete'ing NULL.

Quote:
Original post by thedustbustr
Linux: Using goto In Kernel Code


Heh. IMX, people who defend goto in those sorts of threads generally don't know what they're talking about, and it's generally pretty transparent (at least to me) that this is the case. The last post shown, in particular, is gold:

Quote:


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



Er, you don't get "stack-esque wind and unwind" by calling one function from another (as in my above snippet)? Where does he think he got the term from?! He must be imagining (this is the only explanation I can think of) that every user of the code calls the allocation function, then the implementation function, then the cleanup function afterward. Not very imaginative, hmm?

Further, my technique offers the usual benefits of function decomposition: namely, (a) you can (in some cases; i.e. if your available resource set is a prefix of the total required set, in the order you choose to put them) reuse resources from other places if you happen to have them for some reason (and want to hang onto them afterwards, as you typically would in these cases); and (b) you can ignore the resource allocation code completely while debugging the implementation function.

Although I suppose it's bad form to speak ill of the dead (the thread, not the poster!)...

[Edited by - Zahlman on July 3, 2008 2:56:24 PM]

Share this post


Link to post
Share on other sites
I want to make clear that when i used alloca, i meant alloca and NOT malloc. Malloc you free, alloca you dont. Malloc you can allocate tons of memory, alloca you cant (remember, its on the STACK).

Zahlman:
>Wait, what?

The 'other' time i use gotos, are when i need to break out of 2 loops. The break keyword unfortunately only breaks out of the current loop. I would like a break 2; keyword. (i never needed 3 but why not have it)

Share this post


Link to post
Share on other sites
The enmity towards goto has got to be more tradition than anything else at this point. In the dawn of time goto-dominated spaghetti code was a real problem, and the invention and adoption of structured programming languages was a great step forward.
Now, thirty years later, people still spend large amounts of time discussing whether goto may be the best solution to some specific problem in some specific language, but it doesn't change the fact that either way no one is advocating using them in more than a handful of idiomatic cases poorly handled by the existing control structures so their impact is essentially zero. Not even the basic converts seem to be trying to abuse goto anymore.
I mean honestly on the list of hard or just plain fucked up things in C/C++ goto is just not on the radar..

Oh, by the way there is one classic uncontroversial case where goto is useful: code generators.

Share this post


Link to post
Share on other sites
Quote:
Original post by AcidZombie24
I want to make clear that when i used alloca, i meant alloca and NOT malloc. Malloc you free, alloca you dont. Malloc you can allocate tons of memory, alloca you cant (remember, its on the STACK).

Zahlman:
>Wait, what?

The 'other' time i use gotos, are when i need to break out of 2 loops. The break keyword unfortunately only breaks out of the current loop. I would like a break 2; keyword. (i never needed 3 but why not have it)


I think you've misunderstood. You said you never used them, and then gave an example where you do. I thought that was strange.

I've had the "break N" idea before but I suspect it leads to maintenance nightmares. Then again, maybe that's just the thing to discourage overly deep nesting. ;) You might want to look at how Java handles it: you can do things like


label: for (stuff) {
for (other stuff) {
if (weird thing) { break label; }
}
}


Notice that the label applies to the loop being broken out of, not to where the code flow ends up. Also note that not only is there no 'goto' in Java, they took precautions in the design to avoid accidentally adding it in the future. ;)

Share this post


Link to post
Share on other sites
Quote:
Original post by implicit
The enmity towards goto has got to be more tradition than anything else at this point. In the dawn of time goto-dominated spaghetti code was a real problem, and the invention and adoption of structured programming languages was a great step forward.


I disagree. Goto is, generally speaking, a way of being maliciously lazy: not the good kind of lazy that leads to automating things you don't want to do yourself, but the bad kind of lazy that leads to not communicating your ideas to people who need to hear them.

The reason there are loop constructs in the language is so you can be expressive. Strictly speaking they are totally unnecessary. It is trivial to implement them all in terms of goto and if. We don't because (a) the person reading the code then has to figure out what's going on, and (b) the for/while/etc. form is often easier or shorter to write. In cases where (b) fails, many programmers forget about (a). (And sometimes they are simply mistaken about (b); if you get sufficiently accustomed to certain "goto idioms", you can end up missing very simple alternate structures. I've seen it happen).

Using goto to control flow reflects thinking of the program as a flowchart - with all the symbols in a line, at that. This is a poor model for how we normally approach problems.

Quote:
Now, thirty years later, people still spend large amounts of time discussing whether goto may be the best solution to some specific problem in some specific language, but it doesn't change the fact that either way no one is advocating using them in more than a handful of idiomatic cases poorly handled by the existing control structures so their impact is essentially zero. Not even the basic converts seem to be trying to abuse goto anymore.


There are some pretty egregious (ab)uses in the Linux kernel code. The *actually* "idiomatic" cases are in fact not so "poorly handled by existing control structures", either. This is an excuse made by inexperienced programmers, who either haven't thought about it very hard, or got scared off by a nasty "general-case solution", or both.

I *will* agree with Linus that not having even things like break, and instead being forced to work around them with loop-exit flags, is really horrible. Break, as a concept as well as a keyword, is just fine. The SESE doctrine has IMO very much missed the point.

Quote:
I mean honestly on the list of hard or just plain fucked up things in C/C++ goto is just not on the radar..


Let me tell you a story: when I learned to program, I learned BASIC first, then Turing, then C. BASIC and C have goto; Turing does not. I used C (though very infrequently, while also picking up other much friendlier languages) for years before I actually found out it has goto. My instinctive reaction was "Ewwwww, why would I want to go back to using THAT?" I swear noone had ever previously told me not to use goto at that point. :)

Quote:
Oh, by the way there is one classic uncontroversial case where goto is useful: code generators.


I'm not convinced. If you're smart enough to write code that doesn't use goto, and you're smart enough to write code that writes code, why wouldn't you be smart enough to write code that writes code that doesn't use goto?

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
There are some pretty egregious (ab)uses in the Linux kernel code. The *actually* "idiomatic" cases are in fact not so "poorly handled by existing control structures", either. This is an excuse made by inexperienced programmers, who either haven't thought about it very hard, or got scared off by a nasty "general-case solution", or both.
I honestly haven't actually read more than a small bit of Linux kernel code so I couldn't say but frankly I don't think I've seen anything resembling real goto abuse in code less than a decade old (or possibly written by a newbie).
Seriously, virtually all of it has either been cleanup code as presented here, multi-level breaks, or once in a blue moon jumping into some previous if case to share code where restructuring the loops was tricky. Now I'm not saying that these are necessarily better than the alternatives, merely that they're used so rarely in practice and in such comparatively straightforward ways that it just doesn't matter either way.
I suppose I might have a different attitude if I had maintain ancient C code (and a lot of people do) but from my perspective these eternal goto-wars seem kind of silly.

Quote:
I'm not convinced. If you're smart enough to write code that doesn't use goto, and you're smart enough to write code that writes code, why wouldn't you be smart enough to write code that writes code that doesn't use goto?
Well.. So far my (admittedly feeble) attempts at writing compilers have generally involved translating all control structures into generalized forms of goto, it is after all the textbook way of doing things. Now if you want to spend a bunch of extra code recreating the best-matching C structures to make the code readable or more easily optimized, then that's fine, but in practice you'd rarely bother.
Interestingly I once wrote a static recompiler which relied on tail-recursion and short functions rather than a gigantic goto mess to get extra performance. The downside here is that it wouldn't run in debug mode since the stack exploded without tail-recursion optimizations on.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this