Linked list items wouldn't print

Question

This program is supposed to get user data in the form of a string and put it into a linked list. Right now I am able to get the data into the linked list but not sure why its not printing them out.

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

// define the node of the stack
typedef struct node{
    char name[100];
    struct node *next;
}Node, *NodePtr;

// define the stack based on the linked list of nodes
typedef struct{
    NodePtr top;
}StackType,*Stack;

// implement all the stack operations
Stack initStack(){
    // allocate memory
    Stack sp=(Stack)malloc(sizeof(StackType));
    // set the top pointer to NULL
    sp->top=NULL;
    return sp;
}

int empty(Stack s){
    return (s->top==NULL);
}


void push(Stack s, char *n){
    NodePtr np= (NodePtr) malloc(sizeof(Node));
    strcpy(np->name,n);
    np->next=s->top;
    s->top=np;
}

I think there is a problem in the pop function somewhere but cant seem to figure it out

//  pop the top element from the stack
char* pop(Stack s){
    if(empty(s)){
        printf("\n Error: Stack is empty");
        return("err");
    }
    char hold[100];
    strcpy(hold,s->top->name);
    NodePtr temp=s->top;
    s->top=s->top->next;
    free(temp);
    return hold;
}


int main(){
    char n[100];
    // create the stack
    Stack s=initStack();

    printf("Enter a list of names\n");
    scanf("%s",&n);
    while(strcmp(n,"end")!=0){
        push(s,n);
        scanf("%s",&n);
    }

    // print the stack
    while(!empty(s))
        printf("%s \n ", pop(s));

}

If you can't manage to print the list items then what makes you sure you got the data into the list in the first place? — John Bollinger, Mar 09 '17 at 20:53
In any case, what do you actually see? Compilation error? Runtime error? Incorrect output? Provide details. — John Bollinger, Mar 09 '17 at 20:54
`char hold[100];` is local auto variable. It can not be used outside the scope. — BLUEPIXY, Mar 09 '17 at 20:55
Do not hide pointer nature behind a typedef (`Stack`, `NodePtr`). — John Bollinger, Mar 09 '17 at 20:56
Amplifying what bluepixy said, you're using `return hold`, which returns the address of a variable that is immediately thereafter expiring because the function ends. Though originally written for C++, [this question and the top answer are worth the read](https://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope). — WhozCraig, Mar 09 '17 at 21:02

score 1 · Accepted Answer · answered Mar 09 '17 at 21:52

The function pop returns pointer to a local array that becomes invalid because after exit the function the array is not alive.

Also it is a bad idea when the function pop outputs some messages.

Just rewrite the function the following way

int pop( Stack s, char *item )
{
    int success = !empty( s );

    if ( success )
    {
        strcpy( item, s->top->name ); 

        NodePtr temp = s->top; 
        s->top = s->top->next; 
        free( temp ); 
    }

    return success; 
}

and call it like

while ( pop( s, n ) )
{
    puts( n );
}

Linked list items wouldn't print

1 Answers1