How to push and pop a void pointer in C

Question

I have this working code:

#import <stdlib.h>
#import <stdio.h>

typedef struct myarray {
  int len;
  void* items[];
} MYARRAY;

MYARRAY *collection;

void
mypop(void** val) {
  puts(collection->items[collection->len]);
  *val = collection->items[collection->len--];
}

void
mypush(void* val) {
  int len = collection->len++;
  collection->items[len] = val;
  puts(collection->items[len]);
}

int
main() {
  puts("Start");
  collection = malloc( sizeof *collection + (sizeof collection->items[0] * 1000) );
  collection->len = 0;
  puts("Defined collection");
  mypush("foo");
  puts("Pushed foo");
  mypush("bar");
  puts("Pushed bar");
  char str1;
  mypop((void*)&str1);
  puts("Popped bar");
  puts(&str1);
  char str2;
  mypop((void*)&str2);
  puts("Popped foo");
  puts(&str2);
  puts("Done");
  return 0;
}

It outputs:

Start
Defined collection
foo
Pushed foo
bar
Pushed bar
(null)
Popped bar

bar
Popped foo
�ߍ
Done

It should output this instead:

Start
Defined collection
foo
Pushed foo
bar
Pushed bar
bar
Popped bar
bar
foo
Popped foo
foo
Done

Being new to C I am not really sure what's going on or why the output is "corrupted" like that. It seems though that the double pointer void** allows you to pass in a pointer and get out a value without knowing the type, so yay. But wondering if one could show how this code should be implemented so I can get a feel for how to do such a thing.

Compiled with clang:

clang -o example example.c

---

Update

I've updated my code to reflect the latest answers, but still not sure the malloc of the collection is correct.

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

typedef struct myarray {
  int len;
  void* items[];
} MYARRAY;

MYARRAY *collection;

void
mypop(void** val) {
  --collection->len;
  puts(collection->items[collection->len]);
  *val = collection->items[collection->len];
}

void
mypush(void* val) {
  int len = collection->len++;
  collection->items[len] = val;
  puts(collection->items[len]);
}

int
main() {
  puts("Start");
  collection = malloc( sizeof *collection + (sizeof collection->items[0] * 1000) );
  collection->len = 0;
  puts("Defined collection");
  mypush("foo");
  puts("Pushed foo");
  mypush("bar");
  puts("Pushed bar");
  char *str1;
  mypop((void**)&str1);
  puts("Popped bar");
  puts(str1);
  char *str2;
  mypop((void**)&str2);
  puts("Popped foo");
  puts(str2);
  free(collection);
  puts("Done");
  return 0;
}

Answer 1

There are a few things to fix, but for a beginner that is not bad.

1. pop

You need to decrement first len (your push does correctly post-increment). This is a stack.

void mypop(void** val) {
     puts(collection->items[--collection->len]);
     *val = collection->items[collection->len];
}

Arrays start at 0, so

len = 0;
items[len++] = elem1;  // len is 0 for the assignment then incremented
items[len++] = elem2;  // len is 1 for the assignment then incremented

then to pop values

elem2 = items[--len];  // len is first decremented to 1
elem1 = items[--len];  // len is first decremented to 0

2. str

What you want is a pointer to chars, a char *, for str1 and str2, since pop() will store a pointer, not a single char.

 char *str1;
 mypop((void **)&str1);
 puts("Popped bar");
 puts(str1);
 char *str2;
 mypop((void **)&str2);
 puts("Popped foo");
 puts(str2);
 puts("Done");
 return 0;

That should fix the visibly corrupted display. However there are a few more things of interest

3. Allocation

Your programs runs because your allocation is big, and items being inside the struct, its space is likely covered by the whole allocation. But that makes an assumption (quite likely, to be fair), which could lead to undefined behavior in some situations.

But to be cleaner, since you have two entities to allocate, that needs two allocations

collection = malloc( sizeof *collection );
collection->items = malloc( sizeof(collection->items[0]) * 1000 );

to be both freed later on.

In this case, the structure should be

typedef struct myarray {
  int len;
  void **;
} MYARRAY

Since MYARRAY itself is pretty small, you could also declare it statically

static MYARRAY collection;

4. import

#import is deprecated, please use #include instead.