K&R Exercise 4-6. Add commands for handling variables

Question

For the following exercise (Chapter 4, Ex-4.6) of the Book (K&R 2nd Edition):

Exercise 4-6. Add commands for handling variables. (It's easy to provide twenty-six variables with single-letter names.) Add a variable for the most recently printed value.

I have written this solution. I'd like to know how to "improve" or at least if what I have done could be considered "decent" in some way and can be improved. (The program is not only based on the solution of Exercise 4.6 but from exercises 4.3 to 4.6, which I modified and added as I completed the exercises.)

main.c

#include <stdio.h>
#include <stdlib.h> /* for atof() */
#include <math.h>

#include "calc.h"

#define MAXOP   100 /* max size of operand or operator */
#define MAXVAL  26

/* reverse Polish calculator */
int
main(void)
{
    int type;
    double op2;
    char s[MAXOP];
    double variable[MAXVAL] = {0.0};

    while ((type = getop(s)) != EOF) {
        switch (type) {
        case NUMBER:
            push(atof(s));
            break;
        /**
         *  In exercise 4-5 of the book. I had to add "access" to library functions
         *  like sin, exp, pow, etc...
         *
         *  For this I decided to implement a function called math_f_handling, to
         *  handle it. (see the file "math_f_handling.c", for a more detailed
         *  description of the following functions:)
         *
         *  - math_f_handling()
         *  - match_math_libs()
         */
        case ID:
            math_f_handling(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;
        /**
         *  As specified in Exercise 4-3. of the book, I extended the calculator
         *  by adding the modulus (%) operator and provisions for negative numbers.
         */
        case '%':
            op2 = pop();
            if (op2 != 0.0)
                push(fmod(pop(), op2));
            else
                printf("error: zero divisor\n");
            break;
        /**
         *  In exercise 4-4 of the book. I was supposed to add commands to print the
         *  top element of the stack, duplicate it, swap the two top elements, and a
         *  command to clear the stack. (the implementations of the following functi-
         *  ons are in the file "stack.c")
         *
         *  -peek()
         *  -duptop()
         *  -swap()
         *  -clear()
         */
        case '?':
            peek();
            break;
        case '#':
            duptop();
            break;
        case '~':
            swap();
            break;
        case 'C':
            clear();
            break;
        case '\n':
            printf("\t%.8g\n", pop());
            break;
        default:
            /**
             *  And here is what I've added to handle the variables, as specified
             *  above (Exercise 4-6).
             */
            if ((type >= 'A' && type <= 'Z') || type == '=') {
                                if (type != '=') {
                                        push(variable[type - 'A']);
                }
            } else
                printf("error: unknown command %s\n", s);
            break;
        }
    }
    return 0;
}

calc.h

#ifndef CALC_H_HEADER
#define CALC_H_HEADER

#define NUMBER  '0' /* signal that a number was found */
#define ID  'I' /* identifier */

extern int sp;
extern char buf[];

void push(double);
double pop(void);
void peek(void);
void duptop(void);
void swap(void);
void clear(void);
int getop(char []);
int getch(void);
void ungetch(int);

void math_f_handling(char []);
int match_math_libs(char []);

#endif  /* CALC_H_HEADER */

getch.c

#include <stdio.h>

#define BUFSIZE 100

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

/* getch: get a (possibly pushed back) character */
int
getch(void)
{
    return (bufp > 0) ? buf[--bufp] : getchar();
}


/* ungetch: push character back on input */
void
ungetch(int c)
{
    if (bufp >= BUFSIZE)
        printf("ungetch: too many characters\n");
    else
        buf[bufp++] = c;
}

getop.c

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

#include "calc.h"

/* getop: get next operator or numeric operand */
int
getop(char s[])
{
    int i, c;

    while ((s[0] = c = getch()) == ' ' || c == '\t')
        ;
    s[1] = '\0';
    if (isalpha(c)) {
        i = 0;
        while (isalpha(s[++i] = c = getch()))
            ;
        s[i] = '\0';
        if (c != EOF)
            ungetch(c);
        return ID;
    }
    if (!isdigit(c) && c != '.')
        return c;   /* not a number */
    i = 0;
    if (isdigit(c))     /* collect integer part */
        while (isdigit(s[++i] = c = getch()))
            ;
    if (c == '.')       /* collect fraction part */
        while (isdigit(s[++i] = c = getch()))
            ;
    s[i] = '\0';
    if (c != EOF)
        ungetch(c);
    return NUMBER;
}

stack.c

#include <stdio.h>

#include "calc.h"

#define MAXVAL  100 /* maximun depth of val stack */

int sp = 0;     /* next free stack position */
double val[MAXVAL]; /* value stack */

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


/* pop: pop and return top value from stack */
double
pop(void)
{
    if (sp > 0)
        return val[--sp];
    else {
        printf("error: stack empty\n");
        return 0.0;
    }
}


/*  peek: print the top element of the stack */
void
peek(void)
{
    if (sp > 0)
        printf("\t%.8g\n", val[sp - 1]);
}


/* duptop: duplicate top element of the stack */
void
duptop(void)
{
    double item = pop();
    push(item);
    push(item);
}


/* swap: swap the two top elements */
void
swap(void)
{
    double item1 = pop();
    double item2 = pop();
    push(item1);
    push(item2);
}


/* clear: clear the stack */
void
clear(void)
{
    sp = 0;
}

math_f_handling.c

#include <stdio.h>
#include <string.h>
#include <math.h>

#include "calc.h"

#define DEF_LEN     8   /* default length */

/**
 *  I did not add all the functions found in the book
 *  (Appendix B, Section 4).
 *  
 *  Note: In the math_f_handling function, I had to comment out the line 
 *  that is inside the default statement, because when the user tries to
 *  enter input like the following:
 *      
 *      15 A = 20 B = A B +
 *
 *  The user may receive this result:
 *
 *      A is not a supported function.
 *      B is not a supported function.
 *      A is not a supported function.
 *      B is not a supported function.
 *          35
 */
static const char *math_libs[] = 
    {
        "sin", "cos",
        "tan", "pow",
        "exp", "sqrt",
        "floor", "ceil"
    };

void
math_f_handling(char s[])
{
    double op2;
    int n = match_math_libs(s);
    
    switch (n) {
    case 1:
        push(sin(pop()));
        break;
    case 2:
        push(cos(pop()));
        break;
    case 3:
        push(tan(pop()));
        break;
    case 4:
        op2 = pop();
        push(pow(pop(), op2));
        break;
    case 5:
        push(exp(pop()));
        break;
    case 6:
        push(sqrt(pop()));
        break;
    case 7:
        push(floor(pop()));
        break;
    case 8:
        push(ceil(pop()));
        break;
    default:
        // printf("%s is not a supported function.\n", s);
        break;
    }
}


int
match_math_libs(char s[])
{
    int loc, found;

    found = -1;
    for (loc = 0; loc < DEF_LEN; loc++) {
        if (strcmp(s, math_libs[loc]) == 0) {
            found = loc + 1;
            return found;
        }
    } 
    return found;
}

I included my Makefile, just in case.

CC=gcc
CFLAGS=-Wall -g
LDFLAGS=

SRC=src
OBJ=obj
HDR=hdr
SRCS=$(wildcard $(SRC)/*.c)
OBJS=$(patsubst $(SRC)/%.c, $(OBJ)/%.o, $(SRCS))
HDRS=$(wildcard $(SRC)/*.h)

BINDER=bin
BIN=$(BINDER)/main
SUBMITNAME=project4-6.zip

all: $(BIN)

release: CFLAGS=-Wall -O2 -DNDEBUG
release: clean
release: $(BIN)

$(BIN): $(OBJ) $(OBJS) $(BINDER)
    # $(CC) $(CFLAGS) $(OBJS) -o $@ -lreadline
    $(CC) $(CFLAGS) $(OBJS) -o $@ -lm

$(OBJ)/%.o: $(SRC)/%.c
    $(CC) $(CFLAGS) -c $< -o $@

$(OBJ):
    mkdir -p $@

$(BINDER):
    mkdir -p $@

clean:
    $(RM) -r $(BINDER)/* $(OBJ)/*
    $(RM) -r $(OBJ)
    $(RM) -r $(BINDER)
    $(RM) *.i
    $(RM) *.zip
    $(RM) *.s
    $(RM) *.bc
    $(RM) *.txt
    $(RM) *.out

submit:
    $(RM) $(SUBMITNAME)
    zip $(SUBMITNAME) $(BIN)

these are a few of the tests that I did with the program that I've written.

1 2 - 4 5 + *
    -9

15 A = 20 B = A B +
    35

15 A = 20 B = A B + ?
    35
    35

15 A = 20 B = A B %
    15

3 4 ? ~
    4
    3

5 2 sin
    0.90929743

4 3 cos
    -0.9899925

2 2 pow
    4

Answer 1

Mod or remainder?

"by adding the modulus (%) operator" --> C does not have a mod operator. % is the remainder, as is fmod(). What's the difference between “mod” and “remainder”?.

Why range test?

type >= 'A' && type <= 'Z' is not as portable as isupper(type). C does not require consecutive letters. Example.

Namespace

calc.h declares functions all over the namespace. Consider a more localized naming conventions like calc_peek, calc_pop, .... This will reduced collisions with other code.

char vs. unsigned char

static char buf[BUFSIZE]; better as static unsigned char buf[BUFSIZE]; to avoid implementation defined behavior, UB on is...() functions and cope with rare non-2's complement encoding.

How much precision?

Code uses 8, yet more is needed to well distinguish doubles.

    // printf("\t%.8g\n", val[sp - 1]);
    printf("\t%.*g\n", DBL_DECIMAL_DIG, val[sp - 1]);
    // or fold some nearby value together
    printf("\t%.*g\n", DBL_DIG, val[sp - 1]);

Why only ' ', '\t'

c = getch()) == ' ' || c == '\t' compares only against 2 possible white-spaces. More common to use isspace(c).

Error messages

Consider printing errors on stderr.

// printf("ungetch: too many characters\n");
fprintf(stderr, "ungetch: too many characters\n");

Signed values

I'd expect getop() to accept input with a '-' or '+ sign.

Stack empty?

At the end of processing, the stack should be empty. I'd expect a test for that.

Max operand

As double ranges typically from 1.0e+/-300 or more and code does not support exponential input, to allow input of all double, use at least DBL_MAX_10_EXP, -DBL_MIN_10_EXP. To allow extended precision, leading zeros, spaces, etc., consider 2x DBL_MAX_10_EXP for the max operand size.

Answer 2

Good job on the include guards in calc.h.

In addition to the already posted excellent review I have a few suggestions.

Each C source file should have its own header file, for instance stack.c should have a stack.h header file that provides the function prototypes for the functions push(), pop(), peek(), duptop(), etc. If you do want calc.h to provide all the functions you can then include each header file.

Only include the headers where they are necessary.

An enum might be better than #defines for NUMBER and ID. It would be easier to expand and enums provide numeric values rather than character values.