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;
/* ... */
}
The use of goto in C (not C++)
Author
Say I have this function:
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]
I think goto is OK here, but I think you can simplify the logic just a little bit. Something like this would be better:
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?
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?
Author
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].
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;}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 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:
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)
Author
Quote:Original post by Simian ManQuote: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.
Author
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?
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
Linux Device Drivers, 2nd Edition; Chapter 2: Building and Running Modules; Error Handling in init_module
Quote:Original post by Gage64Quote: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.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
