Thread: Overflow in Stack Counter

Just for your benefit of knowing the proper syntax

Your code:

Code:

float is_empty(stack_type* stk)
{
   if ( (*stk).top == -1 )
      return TRUE;
   else
      return FALSE;
}

That function is fine.

Altered code:

Code:

float is_empty(stack_type* stk)
{
   if ( stk->top == -1 )
      return TRUE;
   else
      return FALSE;
}

Which does the same job, but may be a little easier on the reader. Plus its easier to type out as well.

Originally Posted by mlsrar

Yes, I am learning C. In using double-quotes, I still receive the aforementioned warning during compilation and it segfaults regardless of input.

I wouldn't think I could legally declare

const char* input[STACKSIZE];

If you used double quotes, you did not get the integer-to-pointer warning. (Unless you forgot to save, or are compiling some other program, or you only changed one out of two.)

You can do the second, but let me ask you this question: what do you think it is, and how would you plan to use it?

48 is '0', right? Just ditch the magic number and put '0' instead. You could use isdigit() to clean up some of your code and make an isoperator() function to clean up the rest. Do it.

I am intrigued by the fact that your stack stores numbers as integers, yet one could be mislead to believe it stores floats. Why not just have your stack space as an array of floats?

Compilation with warnings:

calc.c:38: warning: passing arg 1 of `fgets' from incompatible pointer type
calc.c:40: warning: passing arg 1 of `strcmp' from incompatible pointer type
calc.c:40: warning: passing arg 2 of `strcmp' makes pointer from integer without a cast
calc.c:42: warning: passing arg 1 of `strlen' from incompatible pointer type

It runs, but seg-faults:

Code:

int main(int arc, char **argv)
{
   char buffer[256];
   const char* input[STACKSIZE];
   stack_type stk;
   int i=0;
   stk.top = -1;            /* Initialize the stack to empty */

   printf("This program will evaluate postfix expressions using a\n");
   printf("good implementation of stacks.\n");
   printf("You may enter up to a 50-char expression.\n");
   printf("Please enter a postfix/reverse-polish expression for me.\n");
Line 38:   fgets(input, sizeof(input), stdin);

Line 40:   while (strcmp(input, "quit" == 0)) {

Line 42:   for (i=0; i<strlen(input); i++) {
   switch(*input[i]) {
   case '+':
     push((&stk), pop(&stk) + pop(&stk));
 break;
   case '*':
     push((&stk), pop(&stk) * pop(&stk));
   break;
   case '1':
     push(&stk, (*input[i] - 48));
   break;
   case '2':
     push(&stk, (*input[i] - 48));
...
...
...

I appreciate everyone's help and assistance.

Originally Posted by tabstop

If you used double quotes, you did not get the integer-to-pointer warning. (Unless you forgot to save, or are compiling some other program, or you only changed one out of two.)

You can do the second, but let me ask you this question: what do you think it is, and how would you plan to use it?

const char* input[STKSIZE];

Would be a pointer to constant data, the input from the user, which I will need to reference as i pop/push from the stack. I would need to use it to access the elements from the input array individually...i think...

You can work some magic to changle a string literal, but... that is a dirty trick that may break your program. I need to write a tutorial on dirty tricks that definitely will break programs. Yet... the motivation is so low.

It would be an array of pointers to constant data, is what it would be. You can't use this to replace your other input array -- for one thing, these are just pointers; there isn't any room to store actual characters. For another, since they're pointers to constant data, you wouldn't be able to store new data there anyway.

Originally Posted by mlsrar

(PS...any good tutorials on gdb?)

Sure!

GDB Tutorial
GDB Tutorial

Originally Posted by Elysia

Well, of course, you are passing an array of pointers to a function which expects only a pointer.
But what's worse is that you use pointers without initializing them.
You should read this article to understand why this is bad.
http://cpwiki.sourceforge.net/Common...kes_and_errors

Another error is that you cannot change constant data. You tell the compiler you're never going to change the memory the pointers point to, so inputting data into it cannot be done.

Thanks for the wiki...

I do not to change any constant data, so I will safely avoid using a constant. In the example from the wiki: (Thanks btw)

char[] str = "This is my string";
str[5] = 'n';

I'm using

Code:

char input[STACKSIZE];
fgets(input, sizeof(input), stdin);
 case '1':
     push(&stk, (input[i] - '0'));
...

Do I need a pointer at all? I'm just referencing the 'i'th element of the array.

Compilation with no errors...yay! Thanks for the wiki again.

I'm declaring an array of items from the user-input to push on to the stack to calculate. Inside of a for-loop, I believe I have the correct logic:

Code:

  for (i=0; i<strlen(input); i++) {
   switch(input[i]) {
   case '+':
     push(&stk, (input[i] - 48));
     push((&stk), pop(&stk) + pop(&stk));
   break;
   case '-':
     push(&stk, (input[i] - 48));
     push((&stk), pop(&stk) - pop(&stk));
   break;
   case '*':
     push(&stk, (input[i] - 48));
     push((&stk), pop(&stk) * pop(&stk));
   break;
   case '/':
     push(&stk, (input[i] - 48));
     push((&stk), pop(&stk) / pop(&stk));
   break;
   case '1':
     push(&stk, (input[i] - 48));
   break;
   case '2':
     push(&stk, (input[i] - 48));
   break;
   case '3':
     push(&stk, (input[i] - 48));
   break;
   case '4':
     push(&stk, (input[i] - 48));
   break;
   case '5':
     push(&stk, (input[i] - 48));
   break;
   case '6':
     push(&stk, (input[i] - 48));
   break;
   case '7':
     push(&stk, (input[i] - 48));
   break;
   case '8':
     push(&stk, (input[i] - 48));
   break;
   case '9':
     push(&stk, (input[i] - 48));
   break;
   case 48:
     push(&stk, (input[i] - 48));
   break;
   default:
     printf("Invalid Input\n"); }
}


return 0;
int is_empty(stack_type* stk)
{
   if ( (*stk).top == -1 )
      return TRUE;
   else
      return FALSE;
}


/* Push */
void push(stack_type* stk, float element)
{
   if ( stk == NULL )
   {  printf("Warning! Stack pointer is set to NULL.\n");
      exit(1); /* exit the program */
   }

   /* check stack is not full */
   if ( (*stk).top == (STACKSIZE - 1) )
   {  printf("Stack Overflow!\n");
      exit(1);
   }

   /* increment stack top */
   (*stk).top++;

   /* add the new element to the top */
   (*stk).items[(*stk).top] = element;

   return;
}

/* Pop */
float pop(stack_type* stk)
{  float value=0;
   /* Check stack exists */
   if ( stk == NULL )
   {  printf("Warning!  Stack pointer is set to NULL.  Is it really there?\n");
      exit(1);
   }
   /* check that the stack is not empty */
   if (is_empty(stk))
   {  printf("Stack underflow!\n");
      exit(1);
   }

When I execute it, none of the input characters are matching, and they all cascade to the default statement of invalid input. Am I incorrect in my implementation of:

char input[STACKSIZE];
...
fgets(input, sizeof(input), stdin);
...
push(&stk, (input[i] - 48)) and so on?

Yeah, I remember suggesting that a short while ago. Also here:

Code:

int isoperator(int c)
{
  if(c == '+' || c == '-' || c =='/' || c == '*')
    return c;
  return 0;
}

for (i=0; i<strlen(input); i++) { /* this is already kind of nasty, btw... calling strlen() on each pass. Your optimizer may or may not fix that. */

  int op = isoperator(input[i]);

  if(op)
  {
    switch(op)
      case '+':
        ....
  } else if(isdigit(input[i]))
  {
    push(&stk, (input[i] - '0'));
  }
}

You make life too hard on yourself man...