feedback for exercise 4-11 in K&R

Question

I know that there is one more topic about the exercise 4-11, but the difference is that I solved this exercise and i just need some feedback on my solution. So, I'll explain how it works on an output like "3 4 +\n", for example. The static variable c is initialized with the value ' ' so the condition of the first if statement evaluates to true, the while loop within the if statement will run until c reaches the value '3'. Because it is a number, the function will return the NUMBER signal and the variable c will get the value ' ' - the value that breaks the loop within the 3rd if statement.

Now we are, again, on the first if statement and, again, c has the value ' '. The condition is evaluated to true and the loop within the if statement will run until c reaches the value '4'. Again, because '4' is a "number" the function will return the NUMBER signal and c will get the value '+' - the value that breaks the loop within the 3rd if statement.

This time c is equal to '+' and the condition of the first if statement is evaluated to false, so the value '+' will be returned. I saved the value of the variable c in a temporary variable tmp, and initialize tmp with the value of c before changing it to ' '.

Without this movement I'll get an infinite loop.

The program is made of 5 parts:

main.c

#include <stdio.h>
#include <stdlib.h>
#include "calc.h"
#define MAXOP 1001

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

    while((entry = getop(s)) != EOF) {
        switch(entry) {
            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) {
                    push(pop() / op2);
                }
                else {
                    printf("can't divide by 0");
                }
                break;
            case '\n':
                printf("The value is: %f \n", pop());
                break;
            default:
                printf("Unrecognized command %s\n", s);
                break;
        }
    }

    return 0;
}

getch.c

#include <stdio.h>
#define MAXBUF 100

static char buf[MAXBUF];
static int bufp = 0; /* next free position in buffer */

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

void ungetch(int c) {
    if(bufp < MAXBUF) {
        printf("ungetch has been called\n");
        buf[bufp++] = c;
    }   
    else {
        printf("the buffer is full\n");
    }
}

stack.c

#include <stdio.h>
#define MAXSTACK 100

static double stack[MAXSTACK];
static int sp = 0; /* next free position in stack */

void push(double f) {
    if(sp < MAXSTACK) {
        printf("\t--> the value %f has been pushed\n", f);
        stack[sp++] = f;
    }
    else {
        printf("error: the stac is full\n");
    }
} 

double pop(void) {
    if(sp > 0) {
        return stack[--sp];
    }
    else {
        printf("error: the stack is empty!\n");
        return 0.0;
    }
}

calc.h

#define NUMBER '0'
#define MAXLINE 1000 

/*stack related functions */
void push(double f);
double pop(void);

/* output */
int getch(void);
void ungetch(int c);

/* filtration fuctions */
int getop(char s[]);

and getop.c

#include <stdio.h>
#include <ctype.h>
#include "calc.h"

int getop(char s[]) {
    int i, tmp; 
    static int c = ' ';

    if((s[0] = c) == ' ' || c == '\t') {
        while((s[0] = c = getch()) == ' ' || c == '\t') 
            ;
    }

    s[1] = '\0';
    if(!isdigit(c) && c != '.') {
        tmp = c;
        c = ' ';
        return tmp;
    }

    i = 0;
    if(isdigit(c)) {
        while(isdigit((s[++i] = c = getch())))
            ;
    }
    if(c == '.') {
         while(isdigit((s[++i] = c = getch())))
            ;
    }

    s[i] = '\0';

    return NUMBER;
}

Answer 1

Comments on getop() only:

Your initial loop repeats the exit condition twice. It (and subsequent code) also contains double assignments, which are unnecessary and generally better avoided:

if((s[0] = c) == ' ' || c == '\t') {
    while((s[0] = c = getch()) == ' ' || c == '\t')
        ;
}

Here is a simpler version:

while (c == ' ' || c == '\t') {
    c = getch();
}
s[0] = c;

Your loop to read a number is repeated and repetition is normally undesirable. The loop could be extracted into a function and called twice:

static int get_number(char *s)
{
    int c = getch();
    for (; isdigit(c); c = getch()) {
        *s++ = c;
    }
    *s = '\0';
    return c;
}

On end of file, this returns EOF.

Note that you should probably handle invalid input such as "1.2.3"

In main

char entry;
...
while((entry = getop(s)) != EOF) {

entry should really be an int