Saucy Goat
Saucy Goat

Reputation: 1675

Best practice for generic data structure implementation in C

In my adventures implementing generic data structures in C, I've come across a dilemma. For example, in the following code:

void add_something(avl_tree_t * my_tree) {
        int new_element = 123;

        avl_insert(my_tree, (void*)&new_element);
}

int main() {
        avl_tree_t * my_tree = avl_create();

        add_something(my_tree);

        // do stuff

        avl_print(my_tree, function_that_prints_ints);

        exit(0);
}

In which avl_insert is defined as

void avl_insert(avl_tree_t * tree, void * data) {
        avl_node_t * new_node = malloc(sizeof(struct avl_node));

        new_node->data = data;
        // do tree balancing stuff
}

In order for my generic insertion function to work, I have to pass it a void * item to store. However, in order for that to work, in this case I need to pass in the address of the new int item I'm adding so that I can then dereference it to a void *. If I am not mistaken, when we're back in the main function, the memory address in which I stored my new element will be compromised.

One way I looked into to solve this issue is to pass in the size of the things I am storing in the tree as a parameter for avl_create, and then allocating memory for a copy of each element I insert. This works because you don't need the original address or value for whatever you added.

Another thing that works is only using the data structure in the span of a single function, which is obviously not viable.

My question is this: what is the best way to go about storing statically allocated data in a generic data structure, be it basic C types or user made structures?

Thank you in advance.

Upvotes: 1

Views: 811

Answers (2)

ensc
ensc

Reputation: 6984

Use a mix-in style; e.g. do not make data part of the node but the node part of the data:

struct avl_node {
    struct avl_node *parent;
    struct avl_node *left;
    struct avl_node *right;
};

struct person {
    char const *name;
    struct avl_node node;
};

struct animal {
    struct avl_node node;
    int dangerousness;
};

Constructors for animal are like

struct animal *animal_create(double d)
{
    struct animal *animal = malloc(sizeof *animal);

    *animal = (struct animal) {
        .node = AVL_NODE_INIT(),
        .dangerousness = d,
    };

    return animal;
}

The generic AVL tree operations could look like

void avl_tree_insert(struct avl_node **root, struct avl_node *node, 
                     int (*cmp)(struct avl_node const *a, struct avl_node const *b))
{
    /* .... */
}

and a cmp function for animal like

int animal_cmp(struct avl_node const *a_, struct avl_node const *b_)
{
     struct animal const *a = container_of(a_, struct animal, node);
     struct animal const *b = container_of(b_, struct animal, node);

     return a->dangerousness - b->dangerousness;
}

Upvotes: 1

Mark Benningfield
Mark Benningfield

Reputation: 2892

To store pointers to data with automatic storage duration, yes, you would have to know the size of the elements in the container and allocate and copy the pointed-to data.

The simplest way is to just allocate and copy in all cases, optionally using a user-specified clone() or create() function to make deep copies, if necessary. This also entails the use of a user-specified destroy() function to dispose of the copies properly (again, if necessary).

To be able to avoid the allocation, then you have to have some kind of state variable that lets you know if the container should allocate, or just copy the pointer value itself.

Note that this should apply to the container object, not to the individual nodes or elements. If a container stores data in one way or the other, it should store all data that way. See Principle of Least Astonishment.

This is the more complex approach, since you have to be sure to use the correct process for adding and deleting elements based on the state variable. It's ususally much simpler to just make sure you never pass in a pointer to a value with automatic storage duration.

Upvotes: 1

Related Questions