Stack implementation in C I have to solve an exercise from ANSI C book:


  Add the commands to print the top elements of the stack without
  popping, to duplicate it, and to swap the top two elements. Add a
  command to clear the stack.


Here is my implementation in RPN Calculator:

#include &lt;ctype.h&gt;
#include &lt;stdio.h&gt;
#include &lt;stdlib.h&gt;

#define MAXOP 100
#define NUMBER '0'
#define MAXVAL 100
#define BUFSIZE 100

int getop(char []);
void push(double);
double pop(void);
double top(void);
void clear(void);
void swap(void);
void duplicate(void);

int getch(void);
void ungetch(int);

int sp = 0;
double val[MAXVAL];
char buf[BUFSIZE];
int bufp = 0;

int main(void) {
    int type;
    double op2;
    char s[MAXOP];

    while ((type = getop(s)) != EOF) {
        switch (type) {
        case NUMBER:
            push(atof(s));
            break;
        case '+':
            push(pop() + pop());
            break;
        case '*':
            push(pop() * pop());
            break;
        case '-':
            op2 = pop();
            push(pop() - op2);
            break;
        case '/':
            op2 = pop();
            if (op2 != 0.0)
                push(pop() / op2);
            else
                printf("error: zero divisor\n");
            break;
        case '%':
            op2 = pop();
            if (op2 != 0)
                push((int) pop() % (int) op2);
            else
                printf("error: zero divisor\n");
            break;
        case 'c':
            clear();
            printf("stack empty!\n");
            break;
        case 'd':
            duplicate();
            printf("duplicate element\n");
            break;
        case 's':
            swap();
            printf("swap elements\n");
            break;
        case '?':
            printf("top element on the stack: %f\n", top());
            break;
        case '\n':
            printf("\t%.8g\n", pop());
            break;
        default:
            printf("error: unknown command %s\n", s);
            break;
        }
    }
    return 0;
}

void push(double f) {
    if (sp &lt; MAXVAL)
        val[sp++] = f;
    else
        printf("error: stack full, can't push %g\n", f);
}

double pop(void) {
    if (sp &gt; 0)
        return val[--sp];
    else {
        printf("error: stack empty\n");
        return 0.0;
    }
}

double top(void) {
    if (sp &gt; 0)
        return val[sp - 1];
    else {
        printf("error: stack empty\n");
        return 0.0;
    }
}

void clear(void) {
    if (sp &gt; 0) {
        while (--sp &gt; 0);
    } else {
        printf("error: stack empty\n");
    }
}

void duplicate(void) {
    if (sp &gt; 0) {
        int elem = val[sp - 1];
        push(elem);
    } else {
        printf("error: stack empty\n");
    }
}

void swap(void) {
    if (sp &gt; 0 &amp;&amp; (sp - 1) &gt; 0) {
        int elem1 = val[sp - 1];
        int elem2 = val[sp - 2];
        val[sp - 1] = elem2;
        val[sp - 2] = elem1;
    } else {
        printf("error: stack empty\n");
    }
}

int getop(char s[]) {
    int i, c;

    while ((s[0] = c = getch()) == ' ' || c == '\t')
        ;
    s[1] = '\0';
    if (!isdigit(c) &amp;&amp; c != '.' &amp;&amp; c != '-')
        return c;
    i = 0;
    if (c == '-')
        s[++i] = c = getch();
    if (isdigit(c))
        while (isdigit(s[++i] = c = getch()))
            ;
    if (c == '.')
        while (isdigit(s[++i] = c = getch()))
            ;
    s[i] = '\0';
    if (c != EOF)
        ungetch(c);
    return NUMBER;
}

int getch(void) {
    return (bufp &gt; 0) ? buf[--bufp] : getchar();
}

void ungetch(int c) {
    if (bufp &gt;= BUFSIZE)
        printf("ungetch: too many characters\n");
    else
        buf[bufp++] = c;
}


What are your opinions about it? Is it proper and optimal way to implement these functions?
Your clear function uses while (--sp &gt; 0); when sp = 0; does the same thing in faster and clearer way. also consider not throwing an error when clearing an empty stack, because it still behaves as expected.
The condition sp &gt; 0 &amp;&amp; (sp - 1) &gt; 0 is esentially just sp &gt; 1 or sp &gt;= 2, which I would prefer, but that is just a matter of personal style.
Instead of using val[sp-1] to get the top element of the array (in swap and duplicate) use the top() function, as it is a good idea to express your stack operations in terms of more primitive stack operations.
Follwing the concept of the defining operation in terms of primitive operations, duplicate can be written as:

void duplicate(void) {
    push(top());
}


and swap can be written as:

void swap(void) {
    int elem1 = pop();
    int elem2 = pop();
    push(elem1);
    push(elem2);
}


Notice how there is no error checking in these functions,
because push(), pop(), and top() handle the full or empty
stack for us.