rtheunissen
rtheunissen

Reputation: 7435

Stack implementation in C producing confusing results

For a component of an assignment for college we have to implement a stack, which I think I've done well enough for it to be functional. When I'm testing the stack by itself it seems to work fine, however when I'm using it as part of my solution it behaves differently, and I can't figure out why. Can someone please point out my mistake? Thanks.

Edit: I can feel a lot of "how does it behave differently?" comments are on the way, so here's what I've noticed. When running the Testing stack section of main, all the operations execute and print perfectly fine, but when I'm running the second part of main and comment out the testing part instead, the program crashes when I'm trying to push onto the stack - something that didn't fail previously.

main.c

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

struct stackNode {
    char data;
    struct stackNode *nextPtr;
};
typedef struct stackNode StackNode;
typedef StackNode *StackNodePtr;

typedef enum {
    false, true
} t_bool;

void* emalloc(size_t s) {
    void* p = malloc(s);
    if (NULL == p) {
        fprintf(stderr, "Memory allocation failed.\n");
        exit(EXIT_FAILURE);
    }
    return p;
}

void print_array(char* array, size_t n){
    int i;
    printf("[");
    for(i = 0; i < n - 1; i++){
        printf("%c, ", array[i]);
    }
    printf("%c]\n", array[i]);
}


// Determine if c is an operator.
int isOperator(char c) {
    char ops [6] = {'+', '-', '*', '/', '%', '^'};
    int i;
    for (i = 0; i < 6; i++)
        if (ops[i] == c) return true;
    return false;
}

int op_priority(char c) {
    if (c == '+' || c == '-') return 0;
    else if (c == '*' || c == '/') return 1;
    else if (c == '^' || c == '%') return 2;
    return -1;
}


// Determine if the precedence of operator1 is less than, equal to, or greater than
// the precedence of operator2. The function returns -1, 0 and 1, respectively.
int precedence(char op1, char op2) {
    int op1_p = op_priority(op1);
    int op2_p = op_priority(op2);
    if (op1_p < op2_p) return -1;
    else if (op1_p > op2_p) return 1;
    else return 0;
}





// Push a value on the stack.
void push(StackNodePtr *topPtr, char value) { 
    StackNodePtr temp = (StackNodePtr) emalloc(sizeof (StackNode));
    temp->data = value;
    temp->nextPtr = *topPtr;
    *topPtr = temp;
}

// Pop a value off the stack. 
char pop(StackNodePtr *topPtr) {
    StackNodePtr t = *topPtr;
    if (NULL != *topPtr) {
        char c = t->data;
        *topPtr = t->nextPtr;
        free(t);
        return c;
    } else {
        printf("Stack is empty.\n");
        return '\0';
    }
}

// Return the top value of the stack without popping the stack.
char peek(StackNodePtr topPtr) {
    if (NULL != topPtr) {
        return topPtr->data;
    } else {
        printf("Stack is empty.\n");
    }
}

// Determine if the stack is empty.
int isEmpty(StackNodePtr topPtr) {
    if (NULL == topPtr) return true;
    return false;
}

// Prints the stack
void printStack(StackNodePtr topPtr) {
    if (!isEmpty(topPtr)){

    StackNodePtr t = topPtr;
    while (NULL != t) {
        printf("%c\t", t->data);
        t = t->nextPtr;
    }
    printf("NULL\n");
    } else {
       printf("Stack is empty.\n");
    }
}

// Convert the infix expression to postfix notation.
void convertToPostfix(char infix [], char postfix [], int expression_length) {
    printf("At top of cnvToPostfix\n");
    int infix_count = 0;
    int postfix_count = 0;

    ////////////////////////////////////////////
    StackNodePtr *stack;
    push(stack, '(');
    printStack(*stack);
    ////////////////////////////////////////////

    infix[expression_length] = ')';
    while (isEmpty(*stack)) {
        char current = infix[infix_count++];
        if (isdigit(current)) {
            postfix[postfix_count++] = current;
        } else if (current == '(') {
            push(stack, current);
        } else if (isOperator(current)) {
            while (true) {
                char top = peek(*stack);
                if (isOperator(top) && precedence(current, top) >= 0) {
                  postfix[postfix_count++] = pop(stack);
                } else {
                    break;
                }
            }
            push(stack, current);
        }
        else if (current == ')') {
            while (true) {
                char top = peek(*stack);
                if (top == '(') {
                    pop(stack);
                    break;
                } else {
                     postfix[postfix_count++] = pop(stack);
                }
            }
        }
    }
}


int main() {

    printf("Testing stack\n");
    printf("Pushing 1, 2, and 3 onto stack, peeking and popping.\n");
    StackNodePtr *stack;
    push(stack, '1');
    push(stack, '2');
    push(stack, '3');
    printf("Printing stack\n\n");
    printStack(*stack);
    printf("Peek: %c\n", peek(*stack));
    printf("Pop: %c\n", pop(stack));
    printf("Printing stack\n");
    printStack(*stack);


/*
    printf("Enter the infix expression.\n");

    char c;
    char infix [1024];
    int count = 0;
    while ((scanf("%c", &c)) == 1) {
        if ((int) c == 10) break;
        infix[count++] = c;
    }

    printf("The original infix expression is:\n");
    print_array(infix, count);


    char postfix [count];

    convertToPostfix(infix, postfix, count);
    printf("The expression in postfix notation is:\n");
    print_array(postfix, count);
*/
     return 0;
}

Upvotes: 0

Views: 202

Answers (2)

Edmund
Edmund

Reputation: 10819

You're not really initialising your stacks:

StackNodePtr *stack;
push(stack, '(');

It's also potentially confusing having StackNodePtr being a pointer type, and stack being a pointer to that type. You need to be clear in every possible usage how many levels of indirection should be applied.

To start with, imagine passing the new stack firstly to isEmpty:

StackNodePtr *stack;
printf("%d\n", isEmptypush(*stack));

What's isEmpty going to do on the value it is passed?

I think what you want instead is:

StackNodePtr stack = NULL;
push(&stack, '(');

Other uses of stack in that function should similarly be changed from *stack to stack, or stack to &stack.

Upvotes: 2

paxdiablo
paxdiablo

Reputation: 881423

I see at least one immediate problem:

StackNodePtr *stack;
push(stack, '1');

Where is the initialisation for your stack? Use of uninitialised pointers is instant "undefined behaviour" territory.

If you look closely at your push code, you'll see it inserts the new item before the current one but set the new item's nextPtr pointer to the previous (uninitialised) value.

That means, the last item in the stack won't actually point to NULL.

Upvotes: 4

Related Questions