C - array subscript is not an integer

Question

I'm working on a program too pop and push values onto a stack and check (peek) at the top of the stack. If you run my program you can see that I get errors for "array subscript is not an integer" and I was wondering what it mean't by this and how I should proceed to fix this error.

stack.c program:

#include <stdlib.h>
#include <stdbool.h>
#include "stack.h"

static int mockUp = 42;
static int *stackTop = &mockUp;
static int stack[10];


int isstackempty() {
   if(top == -1)
      return 1;
   else
      return 0;
}

int isstackfull() {
   if(top == DEFAULT_STACK_SIZE)
      return 1;
   else
      return 0;
}

int top()
{
   return *stackTop;
}

int pop( int *val)
{
    if(!isstackempty()) {
      val = stack[top];
      top = top - 1;   
      return val;
      printf("%d popped from stack\n", val);
   } else {
      printf("Error. Could not pop value as stack is empty.\n");
   }
}

int push( int val)
{
    if(!isstackfull()) {
      top = top + 1;   
      stack[top] = val;
      printf("%d pushed onto stack\n", val);
   } else {
      printf("%d could NOT be pushed as stack is full.\n", val);
   }
}

int main()
{
    push(1);
    push(2);
    push(3);
    push(4);
    push(5);
    push(6);
    push(7);
    push(8);
    push(9);

    printf("Value at top of stack is %d\n", top());

    while(!isstackempty()) {
      int val = pop();
      printf("Stack value popped %d\n", val);
   }

   return 0;

}

stack.h program:

#define DEFAULT_STACK_SIZE 10

extern void setStackSize( int size); 
extern int getStackSize();
extern void deleteStack();
extern int top();
extern int pop(  int* val);
extern int push( int val);

Answer 1

As has been mentioned several times before, top is a function, not a variable. Beyond that though, you seem to not be certain whether you are considering its address or its content. The construct top = top + 1 suggests that you want to use it as an index, so why is it dereferencing a pointer? Why not just hold the index?

I would recommend removing mockUp, stackTop and top() {...} entirely and adding a static int top = -1 in their place. My reading suggests that this will work as you expect.

Answer 2

You have really a lot of errors in this minimal example.

Have you even tried compiling your code?

int top()
{
    return 0;
}

int main(void)
{
    top = top + 1;
}

GCC returns an error:

a.c:18:6: error: lvalue required as left operand of assignment
  top = 4;

The way you create you stack is awful.

1. Don't use global variables
2. You have to check if your stack is full by knowing how many elements are stored and how much space is left in the stack. You do this by saving this information in a variable, not by looking if the top element is a magic number DEFAULT_STACK_SIZE.
3. Use a struct instead of declaring random variables that have no meaning (like mockUp.

The advantage of using a struct for a data structure like this is that you have all the information you need in the object, you don't need to declare separate variables to keep track of the internal state, and you don't need to declare global variables.

#include <stdio.h>

typedef struct stack
{
    int buffer[10];
    size_t len;
} Stack;

void stack_init(Stack *stack)
{
    if(stack == NULL)
        return;

    stack->len = 0;
}

int stack_is_full(Stack *stack)
{
    if(stack == NULL)
        return 0;

    size_t buffer_size = sizeof stack->buffer / sizeof *stack->buffer;

    if(stack->len >= buffer_size)
        return 1;

    return 0;
}

int stack_is_empty(Stack *stack)
{
    if(stack == NULL)
        return 0;

    if(stack->len == 0)
        return 1;

    return 0;
}

// if val is NULL, user wants to pop without
// getting the value
int stack_pop(Stack *stack, int *val)
{
    if(stack == NULL)
        return 0;

    if(stack_is_empty(stack))
    {
        fprintf(stderr, "stack_pop: stack is empty\n");
        return 0;
    }

    stack->len--;

    if(val)
        *val = stack->buffer[stack->len];

    return 1;
}

int stack_push(Stack *stack, int val)
{
    if(stack == NULL)
        return 0;

    if(stack_is_full(stack))
    {
        fprintf(stderr, "stack_pop: stack is full\n");
        return 0;
    }

    stack->buffer[stack->len++] = val;
    return 1;
}

void stack_print(Stack *stack)
{
    if(stack == NULL)
    {
        puts("(null)");
        return;
    }

    printf("[ ");

    int i;
    // printing array backwards, the top first, the
    // last at the bottom
    for(i = stack->len - 1; i >= 0; --i)
        printf("%d%c ", stack->buffer[i], i ? ',': '\0');

    printf("]\n");

}

int main(void)
{
    Stack stack;

    stack_init(&stack);

    int top_val;

    if(stack_pop(&stack, &top_val))
        printf("top val: %d\n", top_val);


    stack_push(&stack, 1);
    stack_push(&stack, 2);
    stack_push(&stack, 3);
    stack_push(&stack, 4);

    if(stack_pop(&stack, &top_val))
        printf("top val: %d\n", top_val);

    stack_print(&stack);

    stack_push(&stack, 5);
    stack_push(&stack, 6);
    stack_push(&stack, 7);
    stack_push(&stack, 8);

    stack_print(&stack);

    if(stack_pop(&stack, &top_val))
        printf("top val: %d\n", top_val);

    stack_print(&stack);

    stack_push(&stack, 99);
    stack_push(&stack, 100);
    stack_push(&stack, 101);
    stack_push(&stack, 102);
    stack_push(&stack, 103);
    stack_push(&stack, 104);

    stack_print(&stack);

    for(int i = 0; i < 15; ++i)
        stack_pop(&stack, NULL); // ignoring the actual value

    stack_print(&stack);

    return 0;
}

This is a very basic stack, obviously it can be improved. The point I'm trying to show is that by encapsulating all the information about the stack in a structure, it's easy to write functions that will manipulate the whole stack and it's internal state. In main it's very easy to use the stack and you can have multiple stack if you like.

---

This is the output of my example:

stack_pop: stack is empty
top val: 4
[ 3, 2, 1 ]
[ 8, 7, 6, 5, 3, 2, 1 ]
top val: 8
[ 7, 6, 5, 3, 2, 1 ]
stack_pop: stack is full
stack_pop: stack is full
[ 102, 101, 100, 99, 7, 6, 5, 3, 2, 1 ]
stack_pop: stack is empty
stack_pop: stack is empty
stack_pop: stack is empty
stack_pop: stack is empty
stack_pop: stack is empty
[ ]

Answer 3

top is a function, not an integer, you need to do

val = stack[top()];

however, there are other issues you will run into with your implementation.

Answer 4

You use top as an index into stack, but top is defined is as the name of a function, and using the name of a function not followed by () gives you a pointer to that function - which is not an integer and not a valid array subscript. There are other issues with your code, too, btw.