0

So here is my issue, I have been trying to figure this out for the last 5 hours, I have a header file, a tester file, and a c source file. I would really like to understand what is happening and why so I can avoid the issue in the future. The header file declares the struct but does not define it:

typedef struct Stack *StackP;

and in my source file, Stack.c I have defined the stack:

struct Stack
{
  int top;
  int capacity;
  int count;
  ItemT items;
};

where ItemT is defined as char *

in the tester file, the call goes:

StackP stackPtr = newStack();

and what I have for my newStack function located in the c source file is:

StackP newStack(void) {
  struct Stack stack1;
  StackP stackPtr = &stack1;
  (stackPtr->items) = (ItemT)malloc(DEFAULT_CAPACITY*sizeof(ItemT));        

  (stackPtr->top) = -1;
  (stackPtr->capacity) = DEFAULT_CAPACITY;
  (stackPtr->count) = 0;    
  fprintf(stderr, "\nSuccesfully allocated memory to items...\n");

  return stackPtr;
}

now, my push function is:

void pushStack(StackP stackPtr, ItemT item) {           
  if ((stackPtr->count) == (stackPtr->capacity)) {
    fprintf(stderr, "\nERROR: Full stack.\n");
  }
  else {
    stackPtr->items = item;
    fprintf(stderr, "\nSuccessfully pushed %s on to the stack...\n", stackPtr->items);
    (stackPtr->items)++;
    (stackPtr->top)++;
    (stackPtr->count)++;
  }
}

My question is this: Have I don't something wrong in any of these blocks of code.

If I call a function that says:

return (stackPtr->count);

it will return a random set of numbers instead of 0, or 1. For instance, if I push 2 strings to the stack, instead of count being 2, count is 479622 or some other random long number. Why is this happening?

Again, I would like to know what I'm doing wrong and not just correct syntax because I really HAVE to understand this.

3
  • 1
    return stackPtr; do you realize you're returning the address of an automatic variable (struct Stack stack1;), resulting in a dangling pointer that invokes undefined behavior to utilize?
    – WhozCraig
    Jan 22, 2015 at 21:05
  • 1
    In newStack you return a local variable as pointer, which is no good (you should malloc that) - Site note: please remove the C++ tag
    – user2249683
    Jan 22, 2015 at 21:05
  • You should move the definition of the Stack structure to a header file so it can be shared by other source files. Sometimes, the compiler needs to know more details than a forward declaration provides. Jan 22, 2015 at 21:09

1 Answer 1

7

The program has undefined behaviour as it is returning the address of a local variable from a function:

StackP newStack(void) {
  struct Stack stack1;
  StackP stackPtr = &stack1;

  return stackPtr;
}

stack1 no longer exists when newStack exits. stackPtr must point to dynamically allocated memory if it is to exist beyond the scope of the function:

StackP newStack(void) {
  struct Stack stack1;
  StackP stackPtr = malloc(sizeof(*stackPtr));
  if (stackPtr)
  {
  }

  return stackPtr;
}

See Do I cast the result of malloc?

2
  • 1
    @FrankPalmasani keep in mind, you do not need to dynamically allocate your Stack if you change the return type of your function to struct Stack rather than StackP. In other words, returning stack1 and having the function return a struct Stack (not a pointer-to) would be sufficient, so long as you also account for this in the free-logic.
    – WhozCraig
    Jan 22, 2015 at 21:16
  • @hmjd yeah, it may be an unstated outer requirement the structure itself be dynamic, but from what I see, there is currently no reason it must be so. It definitely makes the newStack() implementation shorter (two lines).
    – WhozCraig
    Jan 22, 2015 at 21:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.