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

This topic is 3698 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## 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 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 on other sites
Quote:
 Original post by Simian ManThat 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 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 on other sites
Quote:
 Original post by Gage64And 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 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 on other sites
Quote:
Original post by Simian Man
Quote:
 Original post by Gage64And 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 on other sites
Quote:
 Original post by WanMasterYou 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 on other sites
Quote:
Original post by Gage64
Quote:
 Original post by WanMasterYou 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.

1. 1
2. 2
JoeJ
20
3. 3
frob
19
4. 4
5. 5

• 10
• 10
• 12
• 13
• 9
• ### Forum Statistics

• Total Topics
632199
• Total Posts
3004733

×