What the parent said still applies in your case. You just make a wrapper function that does the cleanup. I have used this pattern many times:
static int foo_impl(int arg, int **resource1, int **resource2) {
*resource1 = (int *)malloc(sizeof(**resource1));
if (*resource1 == NULL) return EXIT_FAILURE;
/* Do something with resource1 (omitted)... */
*resource2 = (int *)malloc(sizeof(**resource2));
if (*resource2 == NULL) return EXIT_FAILURE;
/* Do something with resource1 and resource2 (e.g.,
stick them in a global hash table as key/value pairs,
or whatever, omitted)... */
return EXIT_SUCCESS;
}
int foo(int arg) {
int *resource1;
int *resource2;
int ret;
resource1 = resource2 = NULL;
ret = foo_impl(arg, &resource1, &resource2);
if (ret != EXIT_SUCCESS) {
free(resource2);
free(resource1);
}
return ret;
}
The advantages are that it makes it very explicit which resources need to be cleaned up (making it easier to review to be sure you haven't forgotten one, or forgotten to initialize one, because they're in a nice list), and it's harder to screw up the control flow. You can't get out of the function without passing by the clean-up code, and you can't accidentally fall into the clean-up code without explicitly returning an error.
I know it is just an example to illustrate the point but this seems to have a bug in it: resource2 won't be initialized if the function fail to allocate resource1 so resource2 will be null when the code enters the deallocation condition.
A way to fix it would be to return different failures for every allocation and to use a switch without a break to clean up, something like:
switch (ret) {
case EXIT_SUCCESS:
free(resource2);
case EXIT_FAILURE2:
free(resource1);
case EXIT_FAILURE1:
break;
}
EDIT: added potential fix to the code. Sorry if there is any bug in the fix, my C is a little bit ... rusty.
EDIT2: fixed a bug in the fix. C is hard, let's go shopping.
EDIT3: the explanation for the bug was also wrong. It is not a memory leak for but freeing a null pointer. Fixed too.
