C Stack push function

Question

I try to implement a stack in C but I get a very strange error. For some reason my push function does not work..

typedef struct node
{
    int v;
    struct node* next;
}Node;

void push(Node *stack,int val)
{
    Node *p = (Node *)calloc(1,sizeof(Node));
    p->v = val;
    Node *aux = stack;
    if(aux == NULL)
    {
        stack = p;
        return;
    }
    while(aux->next != NULL)
        aux = aux->next;
    aux->next = p;
}

I initialized my stack with NULL

Node *stack = NULL;

and I call the function something like this

push(stack,value)

L.E. I tried to create a pop function with parameter double pointer but the result is the same as for push.

void pop(Node **l)
{
    if((*l) == NULL)
        return;
    else
    {

        Node *aux,*prev;
        prev = *l;
        aux = prev->next;
        if(aux == NULL)
        {
            free(prev->v);
            free(prev);
            return;
        }
        while(aux != NULL)
        {
            prev = aux;
            aux = aux->next;
        }
        prev->next = NULL;
        free(aux->v);
        free(aux);
    }
}

What error do you get? Is it a compilation or a run-time error? Please edit your question and include the error message. — anatolyg
– anatolyg, Commented Jun 22, 2016 at 12:05
your stack is a local variable, which will not change inside the scope push is called — Peter Miehle
– Peter Miehle, Commented Jun 22, 2016 at 12:07
@Costi Ivan I showed already in my answer how push and pop should look for the stack. — Vlad from Moscow
– Vlad from Moscow, Commented Jun 24, 2016 at 10:18

Vlad from Moscow · Accepted Answer · 2016-06-22 12:49:04Z

First of all stack is a data organization that satisfies the policy LIFO (Last Input First Output). That is a new data is added at the top of the stack and popped from the top of the stack.

You should not add a loop to find the tail of the stack that to add a new data.

And you need to pass the top of the stack by reference. Take into account that function parameters are its local variable. Thus in your function

void push(Node *stack,int val);

you are changing the local variable stack of the function that will be destroyed after exiting the function. The original argument will not be changed.

Also the memory allocation can fail You should report an error some way.

Here is a demonstrative program that shows how functions push and pop can be implemented

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

typedef struct node
{
    int v;
    struct node *next;
} Node;

int push( Node **stack, int val )
{
    Node *p = malloc( sizeof( Node ) );
    int success = p != NULL;

    if ( success )
    {
        p->v = val;
        p->next = *stack;
        *stack = p;
    }

    return success;
}

int pop( Node **stack, int *val )
{
    int success = *stack != NULL;

    if ( success )
    {
        Node *p = *stack;
        *stack = ( *stack )->next;
        *val = p->v;
        free( p );
    }       

    return success;
}

int main(void) 
{
    const int N = 10;
    Node *stack = NULL;

    int i = 0;
    int val;

    while ( i < N && push( &stack, i ) ) i++;

    while ( i != 0 && pop( &stack, &val ) ) printf( "%d ", val );

    printf( "\n" );

    return 0;
}

Its output is

9 8 7 6 5 4 3 2 1 0

Peter Miehle · Accepted Answer · 2016-06-22 12:09:33Z

1

your push function needs a pointer to a stack node.

void push(Node **stack, int val) {
  ...
  *stack = p;
  ....
}

push(&stack, value);

answered Jun 22, 2016 at 12:09

Peter Miehle

6,1083 gold badges44 silver badges58 bronze badges

4 Comments

Costi Ivan Over a year ago

could you check my pop function, please ?

Peter Miehle Over a year ago

@CostiIvan test-question: what happens iff you push() one element and then pop() it. (it should be empty, but when I look at your function, I believe it would not) you should write a function printStack(stack)

Costi Ivan Over a year ago

I did..pop() does nothing to my stack...I do not know why

Peter Miehle Over a year ago

@CostiIvan same answer as in push(): you do not change *l (what an ugly name) if aux is null, *l must be set to null

Trudeau Fernandes · Accepted Answer · 2016-06-22 12:09:08Z

0

You need to pass stack by reference:

void push(Node **stack,int val)
{
    Node *p = (Node *)calloc(1,sizeof(Node));
    p->v = val;
    Node *aux = *stack;
    if(aux == NULL)
    {
        *stack = p;
        return;
    }
    while(aux->next != NULL)
        aux = aux->next;
    aux->next = p;
}

Andcall as push(&stack,value)

answered Jun 22, 2016 at 12:09

Trudeau Fernandes

1716 bronze badges

Comments

lostbard · Accepted Answer · 2016-06-22 12:12:33Z

0

Remember to initialize the next field to NULL (p->next)

Since stack is a pointer being passed to the push function, the pointer value changes in the function don't get reflected outside of the function.

Ie: if in the push function, we set stack = something, the stack pointer outside the function wont be set to that.

The way around that is either pass the stack in as a pointer to the pointer, then you can modify the pointer value)

void push(Node **stack,int val)
{
  Node *p = (Node *)calloc(1,sizeof(Node));
  p->v = val;
  p->next = NULL;
  Node *aux = *stack;
  if(aux == NULL)
  {
      *stack = p;
      return;
  }
  while(aux->next != NULL)
    aux = aux->next;
  aux->next = p;
}

Call it via:

push(&stack, 10);

Or perhaps easier to understand for beginners, return the stack value from the push function.

Node * push(Node *stack,int val)
{
  Node *p = (Node *)calloc(1,sizeof(Node));
  p->v = val;
  p->next = NULL;
  Node *aux = stack;
  if(aux == NULL)
  {
    stack = p;
    return stack;
  }
  while(aux->next != NULL)
    aux = aux->next;
  aux->next = p;

  return stack;
}

Call via:

stack = push(stack, 10);

answered Jun 22, 2016 at 12:12

lostbard

5,3001 gold badge17 silver badges18 bronze badges

1 Comment

harper Over a year ago

calloc initializes to zero. There is no need to explicitlely initialize p->next to NULL.

Collectives™ on Stack Overflow

C Stack push function

4 Answers 4

Comments

4 Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

4 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related