Having trouble pushing elements onto a stack

Question

I'm working on a project where I must take an expression in reverse polish notation, push the integers and operators onto a stack, then pop them out off the stack as they are inserted into a binary search tree.

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

struct snode 
{
  int datum;
  struct snode* bottom;
};

struct tnode
{
  int datum;
  struct tnode* left;
  struct tnode*right;
};

struct snode* 
push(struct snode* stack, int x) {
  struct snode *S = (struct snode*)malloc(sizeof(struct snode));
  S->datum = x;
  S->bottom = stack;
  return S;
}

struct snode* pop(struct snode* stack) {
  struct snode *S;
  if (stack == NULL)
    return NULL;
  S = stack->bottom;
  free(stack);
  return S;
}

int
peek(struct snode* stack){
  return stack->datum;
}


struct tnode*
create_node(int x){
  struct tnode* tmp;
  tmp = (struct tnode*)malloc(sizeof(struct tnode));
  tmp->datum = x;
  tmp->right = NULL;
  tmp->left = NULL;
  return tmp;
}

void
print_table(struct tnode *AST){
  if(AST !=NULL){
    print_table(AST->left);
    printf("%d ", AST->datum);
    print_table(AST->right);
  }
}

struct tnode*
build_tree(struct snode *S)
{
  struct tnode* root;
  if (S==NULL){
    return NULL;
  }else{
    int top = peek(S);
    if (top == 65 || top == 83 || top == 88 || top == 68 || top == 77){
      return create_node(top);
    }else{
      root= create_node(top);
      root->right = build_tree(pop(S));
      root->left = build_tree(pop(S));
      return root;
    }
  }
}

int
main(int argc, const char *argv[])
{

  int i = 1;
  struct tnode *tree = NULL;
  struct snode *stack = NULL;


  while (argv[i]!= NULL){
    stack = push(stack, argv[i][0]);
    i++;
  }

 tree =  build_tree(stack);
 print_table(tree);

 return EXIT_SUCCESS;
}

I feel like this should work. Everything compiles clean. I run it by saying

./project 5 4 A

and what comes out is

135208 135224 135208 135240 135208 135224 135208 0 135208 135224 135208 52 0 53 0 

when it should be

4 65 5

I think this is happening where because of how I'm pushing the elements on to the stack.

EDIT:

I initialized i so that i = 1.

this is the result I am getting.

134480 134496 134480 0 5 4 5 

EDIT2:

I decided to get rid of the atol(argv[i]) and change it to just argv[i][0]. see code.

Now my out put is just

65

Answer 1

Finally figured out what was going on!

Here is the newly modified sections that should make your code work (please view the // <== EDIT comments) :

struct tnode*
build_tree(struct snode *S)
{

  struct tnode* root;
  if (S == NULL)
    return NULL;

  int top = peek(S);

  if (top == 65 || top == 83 || top == 88 || top == 68 || top == 77)
  {
     root = create_node(top);
     S = pop(S); // <== EDIT
     root->right = build_tree(S);


     S = pop(S); // <== EDIT
     root->left = build_tree(S);
     return root;
  } 

  root= create_node(top);

  return root;
}

int
main(int argc, const char *argv[])
{

  int i = 1;
  struct tnode *tree = NULL;
  struct snode *stack = NULL;

  int value = 0;
  while (argv[i]!= NULL)
  {
    if ((value = atoi(argv[i])) == 0) // <== EDIT
        value = argv[i][0];
    stack = push(stack, value);
    i++;
  }


 tree =  build_tree(stack);
 print_table(tree);

 return EXIT_SUCCESS;
}

I have noticed 3 issues that are causing the values that you have.

First issue : The result that you are looking for is 4 65 5, which means that if the input is a number (4, 5, etc) you must use the result of atoi(). If the input is not a number, you must use the ASCII value.

To do this, I have used the following lines :

    if ((value = atoi(argv[i])) == 0) // <== EDIT
        value = argv[i][0];
    stack = push(stack, value);

In my implementation of atoi(), when the conversion fails, atoi() returns 0. Therefore, I check the value of the assignment : if atoi() returned 0, use the ASCII value.

Second issue : The second problem you are having is that the build_tree() function seems to have faulty logic. Since this is a tree, I would expect the character A to be the root and the numbers 4 and 5 to be the branches, like so :

  A
 / \
4   5

Your logic seems to reverse the situation, the way you are setup, the numbers become root values and the letters are branches. Reversing your logic solves this :

  int top = peek(S);

  if (top == 65 || top == 83 || top == 88 || top == 68 || top == 77)
  {
     root = create_node(top);
     S = pop(S); // <== EDIT
     root->right = build_tree(S);


     S = pop(S); // <== EDIT
     root->left = build_tree(S);
     return root;
  } 

  root= create_node(top);

  return root;

Last issue : This is the big one. When you call build_tree(), you pass the result of pop() directly. The problem with that, is that you are not affecting the new root value to S. Because of this :

1. When you call build_tree(pop(S)) the first time, you end up freeing the memory space for S.

2. Later, when you call build_tree(pop(S)) a second time, you end up calling pop(S) on the initial value (the one that has been freed early on).

The solution is to reassign S when you call pop(), and calling build_tree(S) afterwards.

     S = pop(S); // <== EDIT
     root->right = build_tree(S);


     S = pop(S); // <== EDIT
     root->left = build_tree(S);

TL;DR : I reformated your code a bit and the issue should be fixed now. Copy paste and you should be good.