tags:

views:

128

answers:

5

I have two functions:

void free_this(THIS *this)
{
    THIS *this_tmp;
    while (this_tmp = this)
    {
        if (this->str)
            free(this->str);
        this = this_tmp->next;
        free(this_tmp);
    }
}

void free_that(THAT *that)
{
    THAT *that_tmp;
    while (that_tmp = that)
    {
        if (that->id)
            free(that->id);
        that = that_tmp->next;
        free(that_tmp);
    }
}

Since they are very similar I was trying to come up with one function to handle them both. I can already just use a pointer to point to the correct data to free (i.e. point to either str from THIS struct or to id of THAT struct) however I can't figure out how to get around what type of struct is being dealt with since I can't just use a void pointer since void* has no member named 'NEXT'.

Any ideas?

Maybe I should just combine the two structs THIS and THAT into one somehow? here they are:

typedef struct this {
    struct this *next;
    char *str;
} THIS;

typedef struct that {
    struct that *next;
    char *id;
    unsigned short result;
    OTHERTHING *optr;
} THAT;

Could I possibly use the offsetof function somehow to get the next element?

+1  A: 

Depends on the exact structure of THIS and THAT. If they are very similar, especially if str and id have the same offsets, you may be able to merge them into one object.

structure THIS {
    void* str;
    ...
};

structure THIS {
    void* id;     /* is at the same offset as str */
    ...
};

union THAS {
    structure THIS this;
    structure THAT that;
    void* pointer; /* at the same offset as str and id */
};

/* and use it like */
void free_thas(THAS* thas) {
    free(thas->pointer);
    ...
}

If you have a bad feeling about this, your are right. Some small change in THIS may cause THAT to explode and so on. Don't do it.

drhirsch
added my struct definitions above.
i'll give that a try
actually this doesn't really help me as I need to be able to get to the next element in the list which is really the problem here, not freeing the data.
A: 

There are more fancy ways to do what you want - but the following example will suffice.

void free_that(void *mem, int type)
{
    switch(type) {
      case THIS_FLAG: {
        THIS *this = (THIS*)mem;

        for(this; this->str != NULL; this = this->next)
           (void)free(this->str);

        break;
      }

      case THAT_FLAG: {
        THAT *that = (THAT*)mem;

        for(that; that->id != NULL; that = that->next)
           (void)free(that->id);
      }

      default: {
        (void)free(mem);
      }
    }

  return;
}

The more fancy way would be to add a void *mem as the first element in the structure and assign str and id as pointers that point to mem (where you malloc the memory). Doing this allows you to either always free the mem element or free the offset of zero cast to void*.

tracy.brown
I think to OP wanted to avoid the reuse of code.
drhirsch
+1  A: 

You have two different singly linked list types here. You could get around this by creating only a single type:

typedef struct node {
    struct node *next;
    void *data;
} NODE;

and have data point to either a char* (or simply a char) or another struct with the three data fields from THAT. Of course you have to remember to free() the data in your free_node() function.

Thomas
+1  A: 

Yet another way is through some primitive inheritance:

struct node {
   struct node *next;
}

struct this {
   struct node mynode;
   ...
}

struct that {
   struct node mynode;
   ...
}

free_any(struct node *this)
{
    struct node *this_tmp;
    while (this_tmp = this)
    {
        this = this_tmp->next;
        free(this_tmp);
    }
}

This only works if "node" is at the top of the structures, and only allows you to thread one linked-list through these structures.

Also, this doesn't allow you to free anything specific to that type of structure; to do that, you would have to setup a callback function ( either by passing it in the free or in some control structure ) that would get invoked. I would probably instead implement a "pop" function which removes the element from the list, and to free the entire list I would pop off each element and then free them as required.

Chris Arguin
+2  A: 

You could implement the free function with a void * and field offsets. Untested:

void free_either(void *either, size_t other_offset, size_t next_offset)
{
    void *either_tmp;
    while (either_tmp = either)
    {
        free((char *)either + other_offset);

        either_tmp = (char *)either + next_offset;
        free(either);
    }
}

free_either(this,offsetof(THIS,str),offsetof(THIS,next));
free_either(that,offsetof(THAT,id),offsetof(THAT,next));

You could then create macros to replace the old free_this or free_that functions.

Randy Proctor