Doubling Dynamic Stack Array

Question

I have an array used to represent a generic stack.

struct Stack {
    int size;
    int type;
    int capacity;
    void **data; // An array of generic data.
};

Stack *new_stack(int type) {
    Stack *tmp = malloc(sizeof(Stack));
    assert(tmp != NULL);
    tmp->size = 0;
    tmp->type = type;
    tmp->capacity = DEFAULT_CAPACITY;
    tmp->data = calloc(tmp->capacity, type);
    assert(tmp->data != NULL);
    return tmp;
}

Would this be the proper way to double the array while preserving its data?

void realloc_stack(Stack *s) {
    int old_capacity = s->capacity;
    s->capacity *= 2;
    s->data = realloc(s->data, s->capacity);
    memset(s->data + old_capacity, 0, old_capacity);
    assert(s->data != NULL);
}

However, when I try calling this from push_stack() like this:

void push_stack (Stack *s, void *data) {
    if (full_stack(s)) realloc_stack(s);
    s->data[s->size++] = data;
}

I get this problem: basically a bunch of zeroes where the actual numbers should be.

int main() {

    Stack *intStack = new_stack(sizeof(int));

    for (int i = 0; i < 15; ++i) {
        push_stack(intStack, (void*)i);
    }
}

Results:

Printing stack: 
14
13
12
11
10
0
0
0
0
9
8
7
6
1
0

What is the type of `Stack.data`? Remember that `s->data + old_capacity` will be based on that size, not `sizeof(char)`. You're running the risk of a buffer overrun here. — Paul Roub, Jun 24 '14 at 19:46
`tmp->data = calloc(tmp->capacity, type);` <-> `s->data = realloc(s->data, s->capacity);` : this is not consistent in the sense of capacity. — BLUEPIXY, Jun 24 '14 at 20:04
@Raymond You have a struct type `Stack` that you haven't shown. It has a `data` member whose type, and therefor size, we can't see. — Paul Roub, Jun 24 '14 at 20:05
`s->data[s->size++] = data;` : There is no consistency in the `type` and size. — BLUEPIXY, Jun 24 '14 at 20:07
So it should be: s->data = realloc(s->data, s->type * s->capacity); Right? — Mickey, Jun 24 '14 at 20:09
_Right?_ : I think that it should be such that if fit to `calloc`. — BLUEPIXY, Jun 24 '14 at 20:10
`s->data[s->size++] = data;` : It should use the `memcpy` perhaps. — BLUEPIXY, Jun 24 '14 at 20:14
Yeah I was thinking about using memcpy as well. When I tried: memcpy(s->data[s->size++], data, s->type); I got segfault :( — Mickey, Jun 24 '14 at 20:15
`s->data[s->size++] = data;` : `s->data` is memory as type(sizeof(int)) * capacitye . set to sizeof(void*) is funny. — BLUEPIXY, Jun 24 '14 at 20:18
Just a question... Why use an array for a stack? I would imagine using a linkedlist would be easier to manage, and more efficient.. — Tim Z., Jun 24 '14 at 20:18
`s->data[]` is array of `void*` , Rather than using this , So It must be converted address calculation. also `push_stack(intStack, (void*)&i);` — BLUEPIXY, Jun 24 '14 at 20:21
@TimZ. I know linked list is easier to manage, but could you explain why it would be more efficient? I thought arrays were more efficient because of locality of reference. — Mickey, Jun 24 '14 at 20:22
@BLUEPIXY I'm sorry, but I'm having a really hard time following what you're saying. — Mickey, Jun 24 '14 at 20:25
`memset(s->data + old_capacity, 0, old_capacity);` : also address arithmetic to `void *` is UB. it be calculated in (void *) even though the extended specifications are wrong. — BLUEPIXY, Jun 24 '14 at 20:29
So a stack is a data structure where you only put things on the top, and remove things from the top right? Well with a linked list, adding things to the start of the list, or to the end of the list is also really easy. Same concept of removing it from the front or end. The main benefit of using a linkedlist instead of an array is that amount of memory allocated is always perfectly enough. — Tim Z., Jun 24 '14 at 20:36
With an array, if you run out of space. You need to double it (as you're doing now). If you don't fill the array up, you're wasting empty space. With a linkedlist each node will be created when it is needed, and deleted when it is needed. — Tim Z., Jun 24 '14 at 20:36
Correction ：`s->data[] is array of void*` -> `s->data[] is array of void` — BLUEPIXY, Jun 24 '14 at 21:50

BLUEPIXY · Answer 1 · 2014-06-25T23:08:51.707

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

#define DEFAULT_CAPACITY 8

typedef struct Stack {
    int size;    //number of element
    size_t type; //size of type, there are no problems with int
    int capacity;//max number of element
    void *data;  //memory of type * capacity
} Stack;

Stack *new_stack(size_t type) {
    Stack *tmp = malloc(sizeof(Stack));
    assert(tmp != NULL);
    tmp->size = 0;
    tmp->type = type;
    tmp->capacity = DEFAULT_CAPACITY;
    tmp->data = calloc(tmp->capacity, type);
    assert(tmp->data != NULL);
    return tmp;
}

void realloc_stack(Stack *s) {
    int old_capacity = s->capacity * s->type;
    s->capacity *= 2;
    s->data = realloc(s->data, s->capacity * s->type);
    assert(s->data != NULL);
    memset((char*)s->data + old_capacity, 0, old_capacity);
}

static inline int full_stack(Stack *s){//Deleting a "static inline" if you are open to the public as an interface
    return s->capacity == s->size;
}

void push_stack (Stack *s, void *data) {
    if (full_stack(s)) realloc_stack(s);
    memcpy((char*)s->data + s->type * s->size++, data, s->type);
}

void printIntObjectDump(Stack *s){
    int i, *p = s->data;
    for(i=0;i<s->capacity;++i){
        printf("%d\n", p[i]);
    }
}

int main() {
    Stack *intStack = new_stack(sizeof(int));

    for (int i = 0; i < 15; ++i) {
        push_stack(intStack, &i);
    }
    printIntObjectDump(intStack);
    return 0;
}

Hi BLUEPIXY, this worked. Can you explain why we need the char* cast? Thanks for your help! — Mickey, Jun 25 '14 at 22:37
@Raymond ; 1) pointer arithmetic to `void *` is UB(Undefined Behavior). 2) memory location which is based on the size of `void *` does not represent the position of the required memory. It should be based on the size of the `s->type`. (e.g int *p = 0; p + 1 meant 0 + sizeof(int)) — BLUEPIXY, Jun 25 '14 at 22:46
It can be calculated correctly cast to `char*` because `sizeof(char)` is `1` are guaranteed. So it is equal to the specified location. — BLUEPIXY, Jun 25 '14 at 22:58
note : Calculation to `void *` in the GCC is allow as an extension specification. — BLUEPIXY, Jun 26 '14 at 00:01

Mike Boch · Answer 2 · 2014-06-25T12:36:52.627

-1

To expand upon what Paul Roub said in the comments above:

You are calling memset(s->data + old_capacity, 0, old_capacity); Your intent is to write the last half of the array with 0's; however, this is not what is occurring. According to the C++ reference, memset "Sets the first num bytes of the block of memory pointed by ptr to the specified value." You are attempting to fill the array with ints which are 32-bit, not 8. Therefore, you should probably adjust the call to this:

memset(s->data + (old_capacity * s->type), 0, old_capacity * s->type);

This should work and solve your issue. Good luck!

edited Jun 25 '14 at 12:36

answered Jun 24 '14 at 20:38

Mike Boch

141
7

This didn't work, but it did change the output a little bit.Printing stack: 14 13 12 11 10 0 0 0 0 9 8 7 6 1 0 – Mickey Jun 24 '14 at 20:51
Oh wow total brain fart on my end. Sorry about that. I think it should be `s->type` instead of `s->type / 4` in both cases. If that doesn't work, try leaving it as just `s->data + old_capacity` for the start location. I'd try this out myself but I have no c++ compiler on this computer. – Mike Boch Jun 25 '14 at 12:36
Also you may want to change the realloc call to `realloc(s->data, s->capacity * s->type);` too for the same reasons. – Mike Boch Jun 25 '14 at 12:41

Doubling Dynamic Stack Array

2 Answers2