code review request: stack implementation

I'm looking for some input on my stack implementation. Anything goes -- style, structure, functionality. Anything I should have done differently?

stack.h

Code:

---

#include <stdio.h>

typedef int stackElementT;

typedef struct {

  stackElementT *contents;
  int size;
  int count;

} stackT;

void STACKinit(stackT *stack, unsigned int);
void STACKpush(stackT *stack, int value);
int STACKpop(stackT *stack);
void STACKempty(stackT *stack);

---

stack.c

Code:

---

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

void                                                           
STACKinit(stackT *stack, unsigned int size) {
 
  stack->contents = malloc(sizeof(stackElementT) * size);       
  stack->size = size;                                           
  stack->count = 0;                                             
                                                               
}

void                                                           
STACKpush(stackT *stack, const stackElementT value) {           
                                                               
  if (stack->count >= stack->size) {
   
    fprintf(stderr, "Cannot push onto a full stack.\n");       
    exit(1);                                                   
                                                               
  }
                                                               
  stack->contents[stack->count++] = value;                     
                                                               
}

stackElementT                                                   
STACKpop(stackT *stack) {
                                                               
  if (stack->count <= 0) {
   
    fprintf(stderr, "Cannot pop off empty stack.\n");           
    exit(1);                                                   
                                                               
  }
                                                               
  return stack->contents[--stack->count];                       
                                                               
}

void
STACKempty(stackT *stack) {                                     
  STACKinit(stack, stack->size);                               
}

---

I do not see why your header file needs
#include <stdio.h>

you should check the malloc return
exit is too harsh way of notifying the caller that he did something wrong - better return error code

You do not provide function that will free allocated memory

Quote:

---

Originally Posted by vart

exit is too harsh way of notifying the caller that he did something wrong - better return error code

---

How would you do this with functions that already have a return type, i.e. STACKpop?

Quote:

---

You do not provide function that will free allocated memory

---

How is this typically done?

A few things, some personal

* the function names are a little 'odd' and hard to read. stackPop() or stack_pop() would be nicer to use IMO.
* --stack->count I'd use parenthesis to avoid operator precedence... just me --(stack->count). Same for stack->count++ -- I find (stack->count)++ easier to read.
* use inclusion guards in stack.h
* no free()?
* STACKinit(stackT *stack, unsigned int size)... consider using 'size_t size' instead of 'unsigned int size'.
* Check malloc doesn't return NULL before proceeding.

EDIT:

> How would you do this with functions that already have a return type, i.e. STACKpop?
Return something that isn't going to occur -1 for example. Or, give it a success flag (example)... Or put the stackElement as an arg

Code:

---

stackElementT STACKpop(stackT * stack, int * result)
{
    if(stack->count <= 0)
    {
        fprintf(stderr, "Cannot pop off empty stack.\n");
        *result = 1;
        /* garbage */
        return 0;                                                   
    }
   
    *result = 0;
    return stack->contents[--stack->count];                       
}

---

Then you'd only use the return value if result == 0.

> How is this typically done?

Code:

---

ptr = malloc(blah);

/* use ptr */

free(ptr);

---

Thanks for reviewing!

Quote:

---

Originally Posted by zacs7

* use inclusion guards in stack.h

---

What do you mean?

Quote:

---

Originally Posted by cs32

Thanks for reviewing!



What do you mean?

---

http://en.wikipedia.org/wiki/Include_guard

http://faq.cprogramming.com/cgi-bin/...&id=1043284392