4 ms·
> C is the wrong language to dynamically allocate everything. The probability to forget to free it, including in every error path, is much higher in my eyes. T
by cbd1984 12y ago
> C is the wrong language to dynamically allocate everything. The probability to forget to free it, including in every error path, is much higher in my eyes.
There's a trick you can use to make remembering to free it more likely. Some people call it "RAII in C", I like to think of it as "nested allocations and errors": When you allocate something, or do something else which one, can fail, and two, must be undone (freed, closed, etc.), do the allocation and deallocation in properly nested pairs, and use gotos to jump to the code which undoes the last successful allocation if some allocation goes wrong. Basically, you arrange your code in a stack, where the most recent successful allocation is undone first, so you can neatly jump down to the code which undoes all successful allocations and doesn't try to undo the unsuccessful ones.
For example:
int foo(void)
{
int ret = 0; /* Variable we return to indicate
success or failure. Defaults to 0,
which is success. */
FILE *inf;
FILE *outf;
if ((inf = fopen("foo", "rb")) == NULL) {
ret = -1;
goto inf_fail;
}
if ((outf = fopen("bar", "wb")) == NULL) {
ret = -2;
goto outf_fail;
}
/* Do the actual work, now that you know you have
all the resources you need. */
fclose(outf);
outf_fail:
fclose(inf); /* If we jump here, we know we
successfully opened inf,
but not outf. */
inf_fail: /* If we jump here, we didn't actually
open squat, so all we can do is return
the status variable we set above. */
return ret;
}
C++ does basically this automatically, which is called RAII, but in C you have to do it by hand.
- edwintorok 12y agoTrying to figure out control flow with more than one goto label in a function is going to get complicated. I prefer to use `do { } while 0`: int foo(void) { int ret = -1; /* Variable we return to indicate success or failure. Defaults to -1, which is failure. */ FILE *inf; FILE *outf; if (!(inf = fopen("foo", "rb"))) { ret = -1; return ret; } do { if (!(outf = fopen("bar", "wb"))) { ret = -2; break; } do { /* Do the actual work, now that you know you have all the resources you need. Use break if something fails. */ ret = 0; } while(0); fclose(outf); } while(0); fclose(inf); return ret; } If you really must use gotos then just one suffices if you initialize all your variables to NULL: int foo(void) { int ret = 0; /* Variable we return to indicate success or failure. Defaults to 0, which is success. */ FILE *inf = NULL; FILE *outf = NULL; if ((inf = fopen("foo", "rb")) == NULL) { ret = -1; goto foo_fail; } if ((outf = fopen("bar", "wb")) == NULL) { ret = -2; goto foo_fail; } /* Do the actual work, now that you know you have all the resources you need. */ foo_fail: if (outf) { fclose(outf); outf = NULL; } if (inf) { fclose(inf); inf = NULL; } return ret; } P.S. the return code of fclose() should also be checked, as I/O errors might only be reported on close