Not able to create stack .Getting segmentation fault.What to do?

Question

Please help i don't know where is the error.I'm getting segmentation fault.I am using code-blocks as IDE.I am new to programming and this is my first attempt to create linked list. I guess there is problem in my push function but i am not able to find it.

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

typedef struct list
{
    int val;
    struct list* next;
} node;

int main()
{
    node* top;
    top = NULL;
    int i;
    int n,m;
    while(1)
    {
        fflush(stdin);
        printf("Please enter i\n");
        scanf("%d", i);
        switch(i)
        {
            case 1:
            {
                printf("\nenter val");
                scanf("%d", &n);
                top=push(n, top);
            }
            case 2:
            {
                m = pop(top);
                printf("Deleted value is %d", m);
            }
            case 3:
            {
                return 0;
            }
        }
    }
}


node* push(int a,node* s)
{
    if(s==NULL)
    {
        s = malloc(sizeof(node));
        s->val = a;
        s->next = NULL;
        return s;
    }
    else
    {
        node* temp;
        temp = malloc(sizeof(node));
        temp->val = a;
        temp->next = s;
        s = temp;
        return s;
    }
}

node* pop(node* s)
{
    int x;
    node* temp;
    x = s->val;
    printf("deleted value is %d", x);
    temp = s->next;
    s->next = NULL;
    free(s);
    s = temp;
    return s;
}

And add some `break;` s to the switch. (and remove the excessive `{}` ) — joop, May 23 '17 at 11:22
`node* pop(node* s)` --> `int pop(node** s)`, `return s;` --> `return x;` — BLUEPIXY, May 23 '17 at 11:23
Sorry if my edit messed with the `case` brackets, but since they where removed by another external edit I felt like I should keep them as they are in the original code. — grek40, May 23 '17 at 11:28
`fflush(stdin);` is undefined behavior. Per **7.21.5.2 The `fflush` function** of [the C Standard](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf): "If `stream` points to an output stream or an update stream in which the most recent operation was not input, the `fflush` function causes any unwritten data for that stream to be delivered to the host environment to be written to the file; **otherwise, the behavior is undefined.**" — Andrew Henle, May 23 '17 at 11:34
Please [edit] your code to reduce it to a [mcve] of your problem. Your current code includes much that is peripheral to your problem - a minimal sample normally looks similar to a good unit test: only performing one task, with input values specified for reproducibility. — Toby Speight, May 23 '17 at 13:16
@BLUEPIXY ya that double pointer method is good.but i just wrote the printf statement inside the pop function and instead of returning the m, i returned s.Also i made the appropriate changes in main.Now thanks to all u guys my code is working fine now:) — codie, May 23 '17 at 14:06

CIsForCookies · Answer 1 · 2017-05-23T11:43:56.307

You invoke UB 2 times in your current code:

fflush(stdin); see here
scanf("%d", i); You should have used & --> scanf("%d", &i), see here (There are many other examples...)

and this is the reason you get segfault.

After you fix these problems, you should note that compiling your code with warnings will show you that the line m = pop(top); is problematic since m is int and pop(top) returns node*, so I would recommend fixing this too, by adjusting the pop function, or the m datatype.

score 0 · Answer 2 · answered May 23 '17 at 13:10

Some changes to be done, refer following code and follow the comments

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

typedef struct list
{
int val;
struct list* next;
} node;

//Add Prototypes
node* push(int a,node* s);
int pop(node** s);

int main()
{
node* top;
top = NULL;
int i;
int n,m;
while(1)
{
    fflush(stdin);
    printf("Please enter i\n");

    // Send the address 
    scanf("%d", &i);

    switch(i)
    {
        case 1:
        {
            printf("\nenter val");
            scanf("%d", &n);
            top=push(n, top);
        }
        break;// Put break statements after each case
        case 2:
        {
            m = pop(&top);// send address of top to reflect the changes for struct
            if(m==-1){//to indicate stack empty
                printf("stack empty");
                break;
            }
            printf("Deleted value is %d", m);
        }
        break;// Put break statements after each case
        case 3:
        {
            return 0;
        }
    }
}
}


node* push(int a,node* s)
{
if(s==NULL)
{
    s = malloc(sizeof(node));
    s->val = a;
    s->next = NULL;
    return s;
}
else
{
    node* temp;
    temp = malloc(sizeof(node));
    temp->val = a;
    temp->next = s;
    s = temp;
    return s;
}
}

// To effect the changes to the structure use double pointer
int pop(node** s)
{
if(*s == NULL)
{
    printf("Stack empty");
    return -1;
}
int x;
node* temp;
x = (*s)->val;
printf("deleted value is %d", x);
temp = (*s)->next;
(*s)->next = NULL;
free(*s);
*s = temp;
return x;
}

Not able to create stack .Getting segmentation fault.What to do?

2 Answers2