Stack pop operation not working after deleting one element

Question

I implemented stack using linked list. Following is the C program.

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

typedef struct StkElmt_{
    int data;
    struct StkElmt_* next;
}stackelmt;

typedef struct stack_{
    stackelmt* head;
    stackelmt* tail;
    int size;
}stack_t;



void stack_init(stack_t* stack){
    stack->head = NULL;
    stack->tail = NULL;
    stack->size = 0;
}


int stack_insert(stack_t* stack, int data){
    stackelmt* new_element = (stackelmt*)malloc(sizeof(stackelmt));
    new_element->data = data;
    if (stack->size = 0)
        stack->tail = new_element;
    new_element->next = stack->head;
    stack->head = new_element;
    stack->size++;
}
int stack_del(stack_t* stack){
    int data;
    stackelmt* old_element;
    if (stack->size == 0)
        return -1;
    data = stack->head->data;
    old_element = stack->head;
    stack->head = stack->head->next;

    if (stack->size == 1)
        stack->tail = NULL;

    free(old_element);
    stack->size--;
    return 0;
}

void printstack(stack_t* stack){
    stackelmt* point = (stackelmt*)malloc(sizeof(stackelmt));
    for (point = stack->head; point != NULL; point = point->next)
        printf("\n(_%d_)\n  ", point->data);


}


void insertfunction(stack_t* stack, int max){
    int x;
    for (int i=0; i<max; i++){
        scanf("%d", &x);
        stack_insert(stack, x);
    }
    printf("the stack is: \n");
    printstack(stack);
}


void deletefunction(stack_t* stack, int num){
    for (int j=0; j<num; j++)
        stack_del(stack);
    printstack(stack);
}

void utilityfunction(){
    int userinput, maxinput, maxdel;
    stack_t* stack = (stack_t*)malloc(sizeof(stack_t));
    stack_init(stack);
    while (1) {
        printf("\n1 to insert elements in the stack\n2 to delete elements in the stack\n3 to break\n");
        scanf("%d", &userinput);
        switch(userinput){
            case 1:
                printf("\nEnter how many elements you want to push into stack: ");
                scanf("%d", &maxinput);
                insertfunction(stack, maxinput);
                break;
            case 2:
                printf("\nEnter how many elements you want to pop from stack: ");
                scanf("%d", &maxdel);
                deletefunction(stack, maxdel);
                break;
            case 3:
                break;
            default:
                printf("Invalid input\n");
                break;
        }   
    }   
}

int main(){
    utilityfunction();
    return 0;
}

Insertion operation works fine. Example:

Enter how many elements you want to push into stack: 5
1
2
3
4 
5
the stack is: 

(_5_)

(_4_)

(_3_)

(_2_)

(_1_)

But the delete function, when called, pops 'num' number of elements from the stack. But the following happens.

Enter how many elements you want to pop from stack: 3

(_4_)

(_3_)

(_2_)

(_1_)

Although I wanted 3 items to pop from the stack, only one item popped, i.e. 5.

What is the error in the code?

`stackelmt* point = (stackelmt*)malloc(sizeof(stackelmt));` in `printstack()` is causing a memory leak. — MikeCAT, Jul 07 '16 at 08:09
Yes change as @BLUEPIXY has mentioned, if not your program would never terminate — Cherubim, Jul 07 '16 at 11:21

score 2 · Answer 1 · edited May 23 '17 at 12:33

The line

if (stack->size = 0)

in the function stack_insert() will set the size to zero and later stack->size++; will make the size 1.

Then, stack_del() will look at the size and think that the stack has only one element. Popping one element, the stack will seem having no elements, so removal of second element or later won't happen.

You should enable compiler warnings and do comparision

if (stack->size == 0)

instead of the assignment in the function stack_insert().

Also note that the allocated memory in printstack() isn't used and it is just thrown away and is causing memory leak, and that you should stop allocating it there.

One more note: They say you shouldn't cast the result of malloc() in C.

Cherubim · Answer 2 · 2016-07-07T08:51:15.673

In stack_insert fuction if (stack->size = 0) must be if (stack->size == 0)

Reason : Assignment operator = always evaluates to be true, you must instead use the logical equals to operator ==

So, this implies that even if your stack->size is not 0, you are are reassigning it to 0 and you perform stack->tail = new_element;

And make it's return type void as you are not returning anything from the function

So, the following function :

int stack_insert(stack_t* stack, int data)
{
  stackelmt* new_element = (stackelmt*)malloc(sizeof(stackelmt));
  new_element->data = data;
  if (stack->size = 0)
      stack->tail = new_element;
  new_element->next = stack->head;
  stack->head = new_element;
  stack->size++;
}

must be changed to :

void stack_insert(stack_t* stack, int data) //change 1
{
    stackelmt* new_element = (stackelmt*)malloc(sizeof(stackelmt));
    new_element->data = data;
    if (stack->size == 0) //change 2
        stack->tail = new_element;
    new_element->next = stack->head;
    stack->head = new_element;
    stack->size++;
}

Further,

don't use int main() instead use int main(void) as int main() can accept any number of arguments but, here you are not sending any arguments so, int main(void) is appropriate
don't caste the return value of malloc()

for example :

stackelmt* new_element = (stackelmt*)malloc(sizeof(stackelmt));

change it to:

stackelmt* new_element = malloc(sizeof(stackelmt));

Reason : malloc() on successful allocation returns void * type which is automatically converted to the type of pointer it's getting assigned to.

@shiladityabasu if you have any doubts feel free to ask :) and by the way if the answer was helpful you can accept it by clicking the tick mark below the number of votes of the answer — Cherubim, Jul 08 '16 at 02:36

Stack pop operation not working after deleting one element

2 Answers2