Trying to print singly linked list, but can't seem to access node data in C

Question

I have a simple singly linked list, which I'm trying to step through and print the name of the data at each node, in this case the nodes hold fruits which contain a single field, name:

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

struct store
{
    struct list * inventory;
};

struct list
{
    struct node * start; 
    int length;
};

struct node 
{
    struct fruit * data;
    struct node * next;
};

struct fruit 
{
    char name[20];
};

/* set inventory to default vals */
void store_init(struct store * store)
{
    store -> inventory = malloc(sizeof(struct list)); 
    store -> inventory -> start = NULL;
    store -> inventory -> length = 0;   
}

void open_store(struct store * store)
{
    struct node * current;
    struct node * traversal_node;
    int i;
    struct fruit fruits[3];  
    char * fruit_names[3];
    fruit_names[0] = "Apple";
    fruit_names[1] = "Mango";
    fruit_names[2] = "Orange"; 

    /* populate fruits with relevant data */
    for(i=0; i < 3; i++)
    {
        strcpy(fruits[i].name, fruit_names[i]);
    }

    for(i=0; i < 3; i++) 
    {
        current = malloc(sizeof(struct node));

        current -> data = &fruits[i];
        current -> next = NULL;

        if(store -> inventory -> start == NULL) { /* check if no other nodes have been inserted, if so, insert at start */
            store -> inventory -> start = malloc(sizeof(struct node));
            store -> inventory -> start = current;
            store -> inventory -> length++;
        } else  {
            traversal_node = store -> inventory -> start;    /* start at header */

            while(traversal_node->next != NULL) {  /* move to end of list */
                traversal_node = traversal_node -> next;
            }

            traversal_node -> next = current;           /* add node */
            store -> inventory -> length++;
        }       

    }

    printf("%s\n", store -> inventory -> start -> data -> name);

}

void print_inventory(struct store * store)
{
    printf("%s\n", store -> inventory -> start -> data -> name);
}

int main()
{
    struct store store;

    /* intiatlize store */
    store_init(&store);

    open_store(&store);

    print_inventory(&store);

}

The first print statement works, but I get nothing when I try calling print_inventory, and if I try looping through the inventory in that function, I get random characters.

I'll note if I try printf("%s\n", store.inventory -> start -> data -> name); in main, it works fine, but I can't seem to pass my store into the print_inventory function.

What am I doing wrong?

Answer 1

This brings back so many memories! There are a lot of problems with your code, but to make it run, you'll need to fix two things:

1) Make sure that the fruits that you load in the inventory are available whenever you need them. The way you have arranged things, it won't happen. This is because the fruits are dead as soon as open_store returns. This is because they live on the stack of the function, which is kaput as soon as the function returns to the caller. There is a rare chance that you might be able to access them, but that would be relying on an undefined behavior. To fix this, you'll need to do the following:

    struct fruit *fruits[3];  
    char * fruit_names[] = {"Apple", "Mango", "Orange"};
    for(i=0; i < 3; i++) {
        fruits[i] = malloc(sizeof(struct fruit));
        strcpy(fruits[i]->name, fruit_names[i]);
    }

With this change, the fruits will be created in the heap (do familiarize yourself with these concepts if you aren't already), and will be available nice and fresh whenever you need them (as long as the program (process) is running). You'll also need to change:

current -> data = fruits[i]; //& is now gone

And we are done.

2) However, the fruits still won't show up. This is because your print_inventory has a strange implementation, to put it mildly.

void print_inventory(struct store * store)
{
    struct node *item = store -> inventory->start;
    int i;
    for(i = 0 ; i < store -> inventory -> length; i++)  {
        printf("%s\n", item -> data -> name);
        item = item -> next;
    }
}

Should work.

Answer 2

your fruits in the open_store function is an automatic (stack) variable. Later in that function you add its pointer to your linked list.

Then the routine is exited the fruits array goes out of scope and pointers to it become invalid and its content will be overwritten sooner or later. So then you access the pointer of your linked list nodes you read random memory.

You have to allocate dynamic memory for the list data.

    current -> data = malloc(sizeof(struct fruit));
    strcpy(current -> data -> name, fruit_names[i]);