tags:

views:

114

answers:

3

I would like to learn a generic cleanup approach which applies to a scenario like the following. Please remember that this is just a -=SAMPLE=-.

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

int main(void)
{
  unsigned char *uchar1, *uchar2, *uchar3;

  if ((uchar1 = malloc(sizeof(*uchar1) * 10)) == NULL) {
    fprintf(stderr, "Error: malloc(uchar1);\n");

    return 1;
  }

  if ((uchar2 = malloc(sizeof(*uchar2) * 10)) == NULL) {
    fprintf(stderr, "Error: malloc(uchar2);\n");

    free(uchar1);
    return 1;
  }

  if ((uchar3 = malloc(sizeof(*uchar3) * 10)) == NULL) {
    fprintf(stderr, "Error: malloc(uchar3);\n");

    free(uchar1);
    free(uchar2);
    return 1;
  }

  /* do something */

  free(uchar1);
  free(uchar2);
  free(uchar3);
  return 0;
}
+1  A: 

A memory pool?

Here is an example using the APR memory pool:

http://svnbook.red-bean.com/en/1.1/ch08s05.html

http://apr.apache.org/docs/apr/0.9/group_apr_pools.html

Yann Ramin
Thanks for the fast response, from the little I understand of it - it seems like a specific handler. I'm looking for a generic approach for a cleanup, the idea of triggering a cleaning routine in a sane/flexible manner?
Doori Bar
+6  A: 

I think I know what you're getting at. Somehing like the following can make it easier to ensure that you correctly manage resources. I beleive that it is one case where goto needn't be considered harmful.

Edited to reflected Summy0001's excellent observarion and fix return of wrong value.

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

int main(void) 
{ 
  unsigned char *uchar1 = NULL;
  unsigned char *uchar2 = NULL;
  unsigned char *uchar3 = NULL; 
  int result = 0;

  if ((uchar1 = malloc(sizeof(*uchar1) * 10)) == NULL) { 
    fprintf(stderr, "Error: malloc(uchar1);\n");
    result = 1;
    goto CLEANUP0;
  } 

  if ((uchar2 = malloc(sizeof(*uchar2) * 10)) == NULL) { 
    fprintf(stderr, "Error: malloc(uchar2);\n"); 
    result = 1;
    goto CLEANUP1;
  } 

  if ((uchar3 = malloc(sizeof(*uchar3) * 10)) == NULL) { 
    fprintf(stderr, "Error: malloc(uchar3);\n"); 
    result = 1;
    goto CLEANUP2;
  } 

  /* do something */ 


  free(uchar3);
CLEANUP2:
  free(uchar2);
CLEANUP1:
  free(uchar1);
CLEANUP0:
  return result; 
} 
torak
Yes, this is what I'm trying to resolve - but the cleanup should be done properly. (which means, calling cleanup routines of routines that been previously called, not ALL of the cleanup routines). How would you suggest doing that? toggle indicator for each and every routine?
Doori Bar
As I've shown, initialise things to invalid values. Then in the CLEANUP code check the value. If the value is still invalid then the resource doesn't need to be released. On the other hand, if it is valid it does need to be released. For resources where an invalid value isn't known you can use a separate boolean value to track if a resource has been acquired.
torak
Thanks for clarifying
Doori Bar
@Doori: obviously, you do not have only single label. I personally (for sake of example) would have used several labels: one per every goto/free().
Dummy00001
@Dummy00001: Yup, that's what I did :)
Doori Bar
torak
I'm actually thinking of using a 1 label of 'cleanups', with a switch() ... that way all the gotos goto 'cleanups', and prior to each step - use a counter++; which orients the cleanup phase.
Doori Bar
@Doori: Counters suck for the purpose. Plain gotos - with properly named labels - are self documenting. With a counter (== poor imitation of a FSM) one has to think every time "what the count is after that line?"
Dummy00001
+2  A: 

I believe there was a good discussion about it here:

http://stackoverflow.com/questions/3339946/how-to-avoid-long-chain-of-frees-or-deletes-after-every-error-check-in-c/3339958#3339958

karlphillip
Thanks a lot for the reference.
Doori Bar