postfix calculator having trouble with segfaults

i'm still beginning to learn c so please bear with me. i'm trying to create a postfix calculator using linked-list. when i compile, it doesn't show any errors but when it's executed it would show Segmentation Fault. i don't know how to deal with this, please help. here's my code:

Code:

---

---

Code:

---

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


    typedef struct node        {
            int num;
            struct node *next;
    } node;


    void push(int num, node **head);
    int pop(node **head);
    void display(node **head);
    int is_empty();
    int evaluatePostfix();
   
    struct node *head;


    int main()        {
            head = NULL;
        char exp[1000]; // = "5 1 2 + 4 * + 3 -";
            printf("Input expression:\t");
            fgets(exp, 1000, stdin);
            for(int i = 1; i <= strlen(exp); i++) {
                    if(exp[i] == '\n') {
                            exp[i] = '\0';
                    }
                    else if (exp[0] == '\n') {
                                printf("stack is empty\n");
                                exit(0);
                    }
            }
            printf("%s = %d\n", exp, evaluatePostfix(exp));
   
            return 0;
}




    int evaluatePostfix(char* exp)        {
            char * token;
            int counter = 0;
            char temp[256][256];
            token = strtok(exp, " ");
            while(token != NULL)        {
                    strcpy(temp[counter], token);
                    token = strtok(NULL, " ");       
                        counter++;
            }
               
            for (int i = 0; i < counter; i++)        {
                    if (isdigit(*(temp[i]))) {
                            int val = atoi(temp[i]);
                push(val, &head);
                    }
            else  {
                            int val1 = pop(&head);
                int val2 = pop(&head);
                switch (exp[i])        {
                                    case '+':    push(val2 + val1, &head);
                                                    printf("%d\n", (*head).num);
                                                break;
                                    case '-':    push(val2 - val1, &head);    break;
                                    case '*':    push(val2 * val1, &head);    break;
                                    case '/':    push(val1 / val2, &head);    break;
                            }
                    }
        }
        return pop(&head);
    }




    void push (int item, node **head) {
            node *temp;
            node * get_node(int);
            temp = get_node(item);
            temp->next = *head;
            *head = temp;
    }


    node *get_node(int item) {
            node *temp;
            temp = (node*)malloc(sizeof(node));
            if (temp == NULL)
                    printf("\nMemory cannot be allocated");
            temp->num = item;
            temp->next = NULL;
            return(temp);
    }




    int pop(node **head)        {
            int item;
            node *temp;
            item = (*head)->num;
        temp = *head;
            *head = (*head)->next;
            free(temp);
            return(item);
    }
       
    int is_empty(node *temp) {
            if (temp == NULL)
                    return 1;
            else
                    return 0;
    }




    void display(node **head)        {
            node *temp;
            temp = *head;
            if(head == NULL)        {
                    printf("stack is empty\n");
                    return;
            }
            printf("\n");
            printf("=========\n");
            while(temp!=NULL)        {
                    printf("%d\n", (*temp).num);
                    temp = (*temp).next;
            }
            printf("=========\n");
    }

---

i think the problem is with

Code:

---

evaluatePostfix(char* exp)

---

but im not sure how to fix it. i appreciate the help

I suggest that you forward declare your functions near to the top as prototypes. What is this means is that instead of declaring them with parameters left unspecified:

Code:

---

int is_empty();
int evaluatePostfix();

---

You would properly include the parameter list:

Code:

---

int is_empty(node *temp);
int evaluatePostfix(char* exp);

---

Furthermore you would declare get_node there too.

Next, this should be moved to the main function:

Code:

---

struct node *head;

---

After all, some of your functions already have a parameter named head. Make it such that those that don't but which need access to the linked list also have such a parameter.

When you have done is and still have the segmentation fault, use a debugger to find out where it happens. This way, you can be pretty certain that the mistake lies before that point, hence you can start searching properly instead of just wildly guessing that the problem lies with evaluatePostfix.

i tried gdb and it showed this. i have no idea how to fix it. what do you suggest?

Code:

---

Program received signal EXC_BAD_ACCESS, Could not access memory.Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x0000000100000ba5 in pop (head=0x1000010e8) at ask.c:104
104          item = (*head)->num;

---

Try doing some error checking: instead of assuming that you can pop from the stack, check that the stack is not empty. If it is, then the input is invalid so you should handle this.

The switch condition in evaluatePostFix() uses the original array instead of the tokenised one.

A default case is often a good idea, e.g. one that prints "Error: unknown operator".

btw, all identifiers for functions in the Standard Library are reserved.

There is a function exp() in math.h, so exp falls into that category.

Technically, using a reserved identifier in your code results in undefined behaviour.

(If you look at your posted code, exp appears in red just like the other standard functions used in the code.)

changing exp to another variable name did the trick. here's the working code so far:

Code:

---

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


    typedef struct node    {
        int num;
        struct node *next;
    } node;


    void push(int num, node **head);
    int pop(node **head);
    void display(node **head);
    int is_empty(node *temp);
    int evaluate(char *expression);


    struct node *head;


    int main()    {
        head = NULL;
        char expression[1000]; // = "5 1 2 + 4 * + 3 -";
        printf("Input expression:\t");
        fgets(expression, 1000, stdin);
        for(int i = 1; i <= strlen(expression); i++) {
            if(expression[i] == '\n') {
                expression[i] = '\0';
            }
            else if (expression[0] == '\n') {
                printf("empty stack\n");
                exit(0);
            }
        }


        printf("Answer: %d\n", evaluate(expression) );
        return 0;
}




    int evaluate(char* expression)    {
        char * token;
        int counter = 0;
        char temp[256][256];
        token = strtok(expression, " ");
        while(token != NULL)    {
            strcpy(temp[counter], token);
            token = strtok(NULL, " ");   
            counter++;
        }
       
        for (int i = 0; i < counter; i++)    {
            if (isdigit(*(temp[i]))) {
                int val = atoi(temp[i]);
                push(val, &head);
            }
            else  {
                int val1 = pop(&head);
                int val2 = pop(&head);
                switch (*(temp[i]))    {
                    case '+':    push(val2 + val1, &head);    break;
                    case '-':    push(val2 - val1, &head);    break;
                    case '*':    push(val2 * val1, &head);    break;
                    case '/':    push(val1 / val2, &head);    break;
                }
            }
        }
        return pop(&head);
    }




    void push (int item, node **head) {
        node *temp;
        node * get_node(int);
        temp = get_node(item);
        temp->next = *head;
        *head = temp;
    }


    node *get_node(int item) {
        node *temp;
        temp = (node*)malloc(sizeof(node));
        if (temp == NULL)
            printf("\nMemory cannot be allocated");
        temp->num = item;
        temp->next = NULL;
        return(temp);
    }




    int pop(node **head)    {
        int item;
        node *temp;
        item = (*head)->num;
        temp = *head;
        *head = (*head)->next;
        free(temp);
        return(item);
    }
   
    int is_empty(node *temp) {
        if (temp == NULL)
            return 1;
        else
            return 0;
    }




    void display(node **head)    {
        node *temp;
        temp = *head;
        if(head == NULL)    {
            printf("stack is empty\n");
            return;
        }
        printf("\n");
        printf("=========\n");
        while(temp!=NULL)    {
            printf("%d\n", (*temp).num);
            temp = (*temp).next;
        }
        printf("=========\n");
    }

---

Quote:

---

Originally Posted by gemera

btw, all identifiers for functions in the Standard Library are reserved.

There is a function exp() in math.h, so exp falls into that category.

Technically, using a reserved identifier in your code results in undefined behaviour.

(If you look at your posted code, exp appears in red just like the other standard functions used in the code.)

---

That should not be a problem because identifiers for these functions from the standard library have external linkage and are reserved for use as identifiers with external linkage, whereas the identifiers for the two variables named exp from the code in post #1 have no linkage and hence do not violate the rule. On the other hand, it is good practice to avoid such potential conflicts anyway. Consequently, my guess is that it is some other changes that were made, rather than "changing exp to another variable name", that "did the trick".

Quote:

---

Originally Posted by pcyap1

here's the working code so far:

---

As a matter of good style, I suggest that you remove the unnecessary first level indentation: for some reason, starting from the second line, you indented everything by one extra level (except for a closing brace).

Also as a matter of style, you might find it more readable to write temp[i][0] instead of *(temp[i]).

Anyway, are there still problems with your updated code? If so, how does it not work?

pcyap1It totally sounds a good exercise for a new programmers to improve their skills, may you explain what your code does by an example?
I got that if you input a string "4+5+6-7", the output should be 6?(printing the number before the least one)

Quote:

---

Originally Posted by Romyo2

pcyap1It totally sounds a good exercise for a new programmers to improve their skills, may you explain what your code does by an example?
I got that if you input a string "4+5+6-7", the output should be 6?(printing the number before the least one)

---

it's a postfix calculator. so if i input "5 1 2 + 4 * + 3 -" the answer would be 14