Building a Stack with C structs After reading the book from which this example is from I started using APIs like this


typedef struct stack {
    char **items;
    int len;
} stack;

stack *stack_new() {
    stack *out = malloc(sizeof(stack));
    //initialize  the struct
    *out = (stack) {};
    return out;
}

void add(stack *in, char *val) {
    in-&gt;len++;
    in-&gt;items = realloc(in-&gt;items, in-&gt;length * strlen(val));
    in-&gt;items[in-&gt;length-1] = val; //&lt;- actually this would be better with strcpy, but you get the idea
}


problem is that valgrind hates it. I've been trying to fix the various memory leaks that structs similar to this one (and to the dictionary of the link) are creating, until I started wondering if it's a good pattern at all.
Here are some things that may help you improve your code.

Write standard C

The code includes this line:

*out = (stack) {};


However, as my compiler helpfully reminds:


  ISO C forbids empty initializer braces


Fix typos

The struct declares a member variable len but the code refers to a member variable length.  If you're asking for code to be reviewed, it would be good if you invest sufficient time to make it as good as you can get it.

Enhance error checking

Both malloc and realloc can fail, but this code doesn't check for that. If realloc fails, then the following line, will attempt to dereference NULL which will cause a program crash on most systems.

Use calloc if you need the memory zeroed

Rather than calling malloc and then expressly initializing your struct, you can do this in one step by calling calloc instead of malloc.  Doing so reduces stack_new() to a single line:

stack *stack_new() {
    return calloc(1, sizeof(stack));
}


Understand pointers

The major problem with this code is that it seems to confuse the length of a string with the size of a pointer to a string.  When you have a variable declaration such as char **items, it means that you have a pointer to a pointer to char.  So the thing stored in items is a pointer to a string, not a string.  For that reason, it should instead be written like this:

void add(stack *in, char *val) {
    in-&gt;items = realloc(in-&gt;items, (1+in-&gt;len) * sizeof(val));
    if (in-&gt;items) {
        in-&gt;items[in-&gt;len] = val; 
        in-&gt;len++;
    } else {
        in-&gt;len = 0;
    }
}


Note that this only increments len if the reallocation succeeded. If it failed, the stack length is set to zero because the contents have been lost. Also, the allocation is to accomodate sizeof(val) rather than the length of the string because what we're storing is the pointer to the string rather than the string itself.  This also means that there is no copy being made of the string itself -- it's just storing a pointer, so that means that the string must continue to exist for the duration of the stack's lifetime which may not be a good design, depending on your proposed use.  

Provide a full sample program for review

It's useful to provide a small but complete program for a code review where practical (and it's certainly practical here).  That would include both header and functions as well as a simple main showing the intended use.  Here is a version of your program that does not leak memory. You and valgrind can become friends again.

#include &lt;stdio.h&gt;
#include &lt;stdlib.h&gt;
#include &lt;string.h&gt;

typedef struct stack {
    char **items;
    int len;
} stack;

stack *stack_new() {
    return calloc(1, sizeof(stack));
}

void stack_free(stack *in) {
    if (in) {
        free(in-&gt;items);
        in-&gt;len = 0;  // not really needed
        free(in);
    }
}

void add(stack *in, char *val) {
    if (in) {
        in-&gt;items = realloc(in-&gt;items, (1+in-&gt;len) * sizeof(val));
        if (in-&gt;items) {
            in-&gt;items[in-&gt;len] = val; 
            in-&gt;len++;
        }
    }
}

void print(stack *in) {
    if (in) {
        for (int i=0; i &lt; in-&gt;len; ++i) {
            puts(in-&gt;items[i]);
        }
    }
}

int main() 
{
    stack *words = stack_new();
    add(words, "Avoid");
    add(words, "memory");
    add(words, "leaks");
    print(words);
    stack_free(words);
}