C Stack data structure

I have implemented a Stack data structure in C and I would like to know if there is something that could be improved.

Stack using linked list:

```
#include "Stack.h"
#include "stdlib.h"

//Linked List based data structure.
typedef struct Stack {
int data;
int size;
struct Stack *next;
}Stack;

/**
* initStackWithData initializes a new Stack by allocating a new pointer with
* the ammount of memory a Stack needs. Then after checking if the newly
* created stack is NULL or not its values are initialized by using the value
* from the parameters and setting the next Stack it points to to NULL.
* This initial Stack will serve as the bottom of the stack so it will never
* point to another Stack with its next property.
*
* @param val - Value to place in the Stack structures data property.
*
* @returns pointer to the just initialized Stack structure(object).
*/
Stack* initStackWithData(int val) {
Stack newStack = (Stack ) malloc(sizeof(Stack));

if(newStack == NULL) {
return NULL;
}

newStack->data = val;
newStack->size = 1;
newStack->next = NULL;

return newStack;
}

/**
* push is pushing a new value to the top of the stack. Since the Stack
* structure is a LIFO(Last In First Out) the value pushed is stored into a new
* Stack structure and the old head Stack (Top) is getting replace with the new
* Stack created. That makes it so that the new Stack is the new top pointing
* to the old top.
*
* @param **head - Pointer to the pointer of head, we use pointer to pointers to
* make it easier by letting us change the entire head Stack from this function.
* @param val - Value to place in the Stack structures data property.
*/
void push(Stack **head, int val) {
Stack newStack = (Stack ) malloc(sizeof(Stack));

newStack->data = val;
newStack->next = *head;
if (!isEmpty(*head)) {
newStack->size = newStack->next->size + 1;
} else {
newStack->size = 1;
}
*head = newSt

Well, let's first look at your single-linked-list themed as a stack:

• You don't ever use .size, so cut that.

• You could call it slist for single-linked-list.

-
initStackWithData is a bad idea. Let them just declare an empty stack and push on that:

Stack* stack = 0;
push(&stack, val);

-
If you determine that a functions precondition is broken, that is a programmer-error and irrecoverable, thus there's just one thing to do:

abort();

Returning without doing anything or otherwise trying to mask the bug just makes properly diagnosing it unneccessarily difficult.

-
Why don't you return the result of the comparison directly in isEmpty()?

-
If you want to branch on a pointer pointing nowhere / somewhere, use !pointer resp. pointer. Much less boilerplate.

p == NULL  ==>  !p
p != NULL  ==>   p

-
C has something called a for-loop. Use it where appropriate.

void display(Stack* stack) {
    for(; stack; stack = stack->next)
        printf("Stack Data: %i\n", stack->data);
}

-
Do not cast the result of malloc. It's redundant. And avoid using sizeof(type), it's error-prone.

Stack *p = malloc(sizeof *p);

-
You forgot to check the result of malloc in push. Correct that.

Now let's look at your stack implemented with an array:

• You forgot the convenience-typedef so you don't have to type struct all the time.

• A compile-time capacity-restriction to 30 can be inconvenient. Why 30?

• See the malloc and sizeof-advice above.

• I wonder why you hang Struct at the end of all those functions... Didn't leaving out the typedef make you type those letters often enough?

• Silently dropping a pushed value if the magic 30 is too small? Bad idea, I really don't want to debug that ever, see above for failed preconditions.

Either make it dynamically extending, just abort, or at the very least return an error-indication to the caller, so he can do that.

-
If the stack is empty, at least add an assertion for debugging, if you don't want to burden release-mode with checking preconditions.

assert(top >= 0);

If you want to have it checked in release-mode too, either do a conditional abort() or compile with assertions enabled.

StackExchange Code Review Q#119084, answer score: 6