Why `push()` in my stack implementation isn't working correctly?

Question

Zero errors and warnings, if I add printf() inside the push() function, it prints 5 and 7 as I want it to, but when I use it in main() I get 0 and 0.

typedef struct Point {
    double x, y;
} Point;

typedef struct Stack {
    int size;
    Point p[];
} Stack;

void push(Stack* stack, double value1, double value2)
{
    stack->size = stack->size++;
    stack->p[stack->size].x = value1;
    stack->p[stack->size].y = value2;
}

int main(void)
{
    Stack stack;
    Point p;
    stack.size = 0;
    stack.p[stack.size].x = 0;
    stack.p[stack.size].y = 0;
    push(&stack, 5, 7);
    printf("%f, %f", &stack.p[stack.size].x, &stack.p[stack.size].y);
}

Answer 1

Zero errors and warnings

Use a better compiler, or learn how to use yours (unfortunately many compilers run in low-warning mode by default).

$ gcc -O -Wall -Wextra -Werror a.c
a.c: In function ‘push’:
a.c:12:17: error: operation on ‘stack->size’ may be undefined [-Werror=sequence-point]
   12 |     stack->size = stack->size++;
      |     ~~~~~~~~~~~~^~~~~~~~~~~~~~~
a.c: In function ‘main’:
a.c:25:5: error: implicit declaration of function ‘printf’ [-Werror=implicit-function-declaration]
   25 |     printf("%f, %f", &stack.p[stack.size].x, &stack.p[stack.size].y);
      |     ^~~~~~
a.c:25:5: error: incompatible implicit declaration of built-in function ‘printf’ [-Werror]
a.c:1:1: note: include ‘<stdio.h>’ or provide a declaration of ‘printf’
  +++ |+#include <stdio.h>
    1 | typedef struct Point {
a.c:25:14: error: format ‘%f’ expects argument of type ‘double’, but argument 2 has type ‘double *’ [-Werror=format=]
   25 |     printf("%f, %f", &stack.p[stack.size].x, &stack.p[stack.size].y);
      |             ~^       ~~~~~~~~~~~~~~~~~~~~~~
      |              |       |
      |              double  double *
a.c:25:18: error: format ‘%f’ expects argument of type ‘double’, but argument 3 has type ‘double *’ [-Werror=format=]
   25 |     printf("%f, %f", &stack.p[stack.size].x, &stack.p[stack.size].y);
      |                 ~^                           ~~~~~~~~~~~~~~~~~~~~~~
      |                  |                           |
      |                  double                      double *
a.c:20:11: error: unused variable ‘p’ [-Werror=unused-variable]
   20 |     Point p;
      |           ^
cc1: all warnings being treated as errors

Let's see what the errors mean.

a.c:12:17: error: operation on ‘stack->size’ may be undefined [-Werror=sequence-point]
   12 |     stack->size = stack->size++;

stack->size++ means add one to stack->size, but if the value of this expression is used, it's the old value of stack->size. You use this expressions in an instruction that also assigns to stack->size. Generally speaking, when there are two assignments to the same variable or memory location (e.g. a structure field or an array element) in the same instruction, the behavior is undefined. “Undefined behavior” means that anything can happen. Some plausible behaviors include:

• This doesn't modify stack->size at all, because the compiler generated code for: 1. read old value; 2. store the result of ++; 3. assign with =.
• This adds 1 to stack->size, because the compiler generated code for: 1. read old value; 2. assign with =; 3. store the result of ++.
• This adds 2 to stack->size, because the compiler generated code for: 1. read old value for ++; 2. store the result of ++; 3. read old value for =; 4. store the result of =.

But other weirder behaviors are possible, including:

• Using different values for stack->size below, because the compiler has inconsistent knowledge about what the value of stack->size is after this instruction.
• The compiler deciding not to generate any code for the push function because since it's attempting to do something that's forbidden, the function must be called only in some particular case that can't happen with these particular build options.
• Demons come out of your nose (if you have an adequate peripheral on your computer).

You obviously meant to add 1 to stack->size. So write either

++stack->size;

or

stack->size++;

or

stack->size += 1;

But only one of these. Don't mix them.

a.c:25:5: error: implicit declaration of function ‘printf’ [-Werror=implicit-function-declaration]
   25 |     printf("%f, %f", &stack.p[stack.size].x, &stack.p[stack.size].y);
      |     ^~~~~~
a.c:25:5: error: incompatible implicit declaration of built-in function ‘printf’ [-Werror]
a.c:1:1: note: include ‘<stdio.h>’ or provide a declaration of ‘printf’
  +++ |+#include <stdio.h>

That one's easy: the compiler gives you the answer. Include stdio.h. The bit about “incompatible implicit declaration” is because since the declaration of printf was missing, the compiler guessed one, but it guessed wrong. The guessing rules date back from half a century ago and are not particularly useful, never rely on them.

a.c:25:14: error: format ‘%f’ expects argument of type ‘double’, but argument 2 has type ‘double *’ [-Werror=format=]
   25 |     printf("%f, %f", &stack.p[stack.size].x, &stack.p[stack.size].y);
      |             ~^       ~~~~~~~~~~~~~~~~~~~~~~
      |              |       |
      |              double  double *

Here the compiler tells you what's wrong, and it should be easy to figure out what to do. To print a floating-point number, pass a floating-point value, not a pointer to one. So:

    printf("%f, %f", stack.p[stack.size].x, stack.p[stack.size].y);

a.c:20:11: error: unused variable ‘p’ [-Werror=unused-variable]

That one means what it says. You declared a variable p but never use it. Just remove its declaration.

With the corrections above, your program might work, although there's one more problem that the compiler can't detect.

typedef struct Stack {
    int size;
    Point p[];
} Stack;
…
Stack stack;

This reserves zero entries in memory for Stack.p. It might work for you if your compiler decided to place the variable Point p immediately after Stack stack in memory, but that's heavily dependent on the compiler, on the presence of other variables, on the variable names, etc. You can't use a flexible array member like Point p[] in a local variable, only in memory allocated with malloc (in most cases). To keep things simple, start with a statically allocated size for the stack.

#define MAXIMUM_STACK_SIZE 42
typedef struct Stack {
    int size;
    Point p[MAXIMUM_STACK_SIZE];
} Stack;

Then your stack can grow up to that many elements.

Answer 2

You do not initialize the flexible array member of your stack.

Change:

stack->size = stack->size++;

which invokes Undefined Behavior, to:

stack->size++;

Moreover, you need to remove the &s from printf("%f, %f", &stack.p[stack.size].x, &stack.p[stack.size].y);.

Putting everything together, we get:

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

typedef struct Point {
    double x, y;
} Point;

typedef struct Stack {
    int size;
    Point p[];
} Stack;

void push(Stack* stack, double value1, double value2)
{
    stack->size++;
    stack->p[stack->size].x = value1;
    stack->p[stack->size].y = value2;
}

int main(void)
{
    // 1 for 1 Point
    Stack *stack = malloc(sizeof(Stack) + sizeof(Point) * 1);
    stack->size = 0;
    stack->p[stack->size].x = 0;
    stack->p[stack->size].y = 0;
    push(stack, 5, 7);
    printf("%f, %f", stack->p[stack->size].x, stack->p[stack->size].y);
    
    free(stack);
    return 0;
}

Output:

5.000000, 7.000000

Read more in https://wiki.sei.cmu.edu/confluence/display/c/DCL38-C.+Use+the+correct+syntax+when+declaring+a+flexible+array+member

Answer 3

Do you really need a flexible array member?

If not, then you could declare your Point array struct data member as a pointer, and after taking care of incrementing size of stack and printing without references as I saw in my other answer, then use malloc() to dynamically allocate memory for your array (in this example I only allocate for 1 Point, but you can easily generalize it), like this:

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

typedef struct Point {
    double x, y;
} Point;

typedef struct Stack {
    int size;
    Point* p;
} Stack;

void push(Stack* stack, double value1, double value2)
{
    stack->size++;
    stack->p[stack->size].x = value1;
    stack->p[stack->size].y = value2;
}

int main(void)
{
    Stack stack;
    // 1 for 1 Point
    stack.p = malloc(1 * sizeof(Point));
    stack.size = 0;
    stack.p[stack.size].x = 0;
    stack.p[stack.size].y = 0;
    push(&stack, 5, 7);
    printf("%f, %f", stack.p[stack.size].x, stack.p[stack.size].y);

    free(stack.p);
    return 0;
}

Output:

5.000000, 7.000000