It's good overall, but I do have a few suggestions.
(Looks like William Morris beat me to a few of these, but there's a few different ones too :p.)
Rather than using the top node as the container, I'd be tempted to make a
Stack that then contains
size_t size; /* example of meta-data */
This would allow passing around a structure and letting functions modify it rather than passing an item by double reference and having the function modify your pointer.
Also, you get the added bonus of stack-related meta data. For example, suppose you wanted to generalize this stack. You would need to store data about how big each element is. With this structure, that's simple. You just throw in another field (and of course put handling for it in the initialization and whatnot). In the
StackItem** version, you're suddenly stuck repeating that information in every instance of
Oh, this also means your sentinel node is no longer required, which can be kind of nice (though it can also complicate some algorithms).
Your methods should return error codes. What if malloc fails in
stack_push? What if
Printing errors in non-user interaction code tends to be bad. Printing communicates with the user, not the code. Your code has no idea stack_pop failed, only your user does. If you pass an error message to the calling code, that code can then choose what to do. It might silently ignore the error, it might alert the user, or it might decide to have a forced freak out. Who knows. The point though is that the calling code needs to be able to decide for itself what to do. Sometimes just alerting the user isn't sufficient.
Other than fully printing or manually handling the struct traversal, how do you get the top element of the stack? Either
stack_pop should handle this, or there should be a
int stack_pop(struct StackItem* current, int* popped); for example.)
There's a some problems with your IO in
First, an int can be 10 characters. This means your
input buffer needs to be 11 characters (10 +
More importantly though,
gets should be avoided. What if instead of a wellformed int or a string <= 10 characters, I decide to put in
aaaaaaaaaaaaaa. Best case, a segfault just happened. Worst case, the stack frame just got modified in a very odd (and potentially insecure) way. (On modern operating systems, the segfault option is almost certainly going to happen, but this should still be avoided at all costs.)
Anyway, I would replace gets with fgets. That way you get the safety of specifying buffer size.
It's way overkill in this situation, but if you decide to go the library-esque route, I would encapsulate your data in a way that the calling code never has to know what's in the struct behind the scene. (This is called an opaque data type). This way your calling code just has to know how to use the exposed API, not the nitty-gritty details.
This explains it pretty well.
All in all, if you decide to go the opaque route, I would have an API similar to:
int stack_init(Stack* s);
int stack_destroy(Stack* s);
int stack_push(Stack* s, int val);
int stack_pop(Stack* s, int* val);
size_t stack_size(void); /* so you can alloc Stacks -- Stack* s = malloc(stack_size()); */
int stack_top(const Stack* s, int* val); /* optional */