Question

how simplify freeing many pointer in this c code using macro :

void main() {
    char *a, *b, *c, *d, *e;

    a = strdup ("test1");
    b = strdup ("test2");  
    c = strdup ("test3"); 
    d = strdup ("test4"); 
    e = strdup ("test5"); 
    f = strdup ("test6"); 

     strcpy(a, "");
     strcpy(e, "");
     strcpy(f, "");


    if(!strcmp(a, "")) 
       free(a);
    if!strcmp(b, ""))
       free(b);
    if(!strcmp(c, ""))
       free(c);
    if(!strcmp(d, ""))
       free(d);
    if(!strcmp(e, ""))
       free(e);
    if(!strcmp(f, ""))
       free(f);
}
Was it helpful?

Solution

Your code is broken, the comparisons against "" make no sense.

You should just free() all of them; if any allocation failed the pointer will be NULL but passing NULL to free() is fine. There is no need to check at the application level.

I assume that your code is some kind of fragment meant to illustrate a point, rather than actual code. If so, then of course you should remove the whole strdup()/free() dance and just use constant strings:

const char *a = "test1", *b = "test2" /* and so on */;

UPDATE After the question has been edited, it makes even less sense to me. It's not a good idea to ever strcpy() into a string returned by strdup(), since you don't know the length of the buffer there's no way to make the copy safe. Also, even if you do overwrite the string with an empty string, you still (of course) own the memory and should call free() on it.

That said, perhaps you're looking for something like this:

#define MAYBE_FREE(s)    do { if(s != NULL && s[0] != '\0') free(s); } while(0)

The wrapping in a do/while loop makes the macro behave as a statement, so you can do:

MAYBE_FREE(a);
if(something())
  MAYBE_FREE(b);

and so on.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top