C: Variable is uninitialized for linked list

Question

I'm relatively new to C and I was creating a program that involves a linked-list. Here is a very abbreviated version of the code that's giving me trouble.

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

#define STRLEN 100

struct Gene { 
    int num[4];
    struct Gene *next;
    };
typedef struct Gene item;

void build_list(item *current, item *head, FILE *in);

int main() {

    FILE *input;
    FILE *output;
    input = fopen("test.data", "r");
    output = fopen("test.out", "w+");

    item *curr;
    item *head; 
    head = NULL;
    int i;

    build_list(curr, head, input);
    curr = head;

    while(curr) {
        for (i = 0; i < 4; ++i)
            fprintf(output, "%d\n", curr->num[i]);
        curr = curr->next;
        }

    fclose(input);
    fclose(output);
    free(curr);
}

void build_list(item *current, item *head, FILE *in) {

    char gene[STRLEN];
    char *tok;
    char gene_name[STRLEN];
    char *search = ",";
    int j;

    while (fgets(gene, sizeof(gene), in)) {

        current = (item *)malloc(sizeof(item));
        tok = strtok(gene, search);
        strcpy(gene_name, tok);
        for (j = 0; j < 4; ++j) {
            tok = strtok(NULL, search);
            current->num[j] = atoi(tok);
            }
        current->next = head;
        head = current;
    }
}

When I try to compile this, it says variable curr is uninitialized, but even when I initialize it with malloc it throws a segmentation fault, or it prints out nothing at all. Why could this be?

You are aware that `build_list(curr, head, input)` passes an uninitialized value as the first parameter? — user253751, Jul 12 '16 at 13:00

Sourav Ghosh · Answer 1 · 2016-07-12T13:13:28.970

2

C uses pass by value for function argument passing. So, when you call build_list(curr, head, input);, curr and head themselves are passed by value and any changes made to those variables (corresponding parameters) will not reflect back to the caller.

So, in the caller,

 while(curr)

is accessing unitialized variable (meeory) which invokes undefined behavior.

If you need to change curr and head themselves, you need to pass their address and make chages inside the funtion. Something like

 build_list(&curr, &head, input);

and

void build_list(item **current, item **head, FILE *in)

and

*current = malloc(sizeof(item));

may get the job done for you.

edited Jul 12 '16 at 13:13

answered Jul 12 '16 at 13:03

Sourav Ghosh

133,132
16
183
261

I did what you recommended, and it brought up a whole bunch of errors like 'member reference base type 'item *' (aka 'struct Gene *') is not a structure or union' and it still warns about the uninitialized variable. What went wrong? – matnnar Jul 12 '16 at 13:12
@matnnar sorry, I did a mistake in the function call example, please check now. – Sourav Ghosh Jul 12 '16 at 13:14
Thanks, but it's still throwing 'member reference base type 'item **' (aka 'struct Gene **') is not a structure or union' as an error on lines 55 and 57. Could you explain to me how I could fix that? – matnnar Jul 12 '16 at 13:16
@matnnar I don't know which is line 55 and 57. and did you change all the variables accordingly? – Sourav Ghosh Jul 12 '16 at 13:17
I...think so? 55 is `current->num[j] = atoi(tok);` and 57 is `current->next = head`. – matnnar Jul 12 '16 at 13:21
@matnnar nope, you're not, remember, we changed the definition (type) of `current` and `head`? Please re-check the data types used. – Sourav Ghosh Jul 12 '16 at 13:22
@matnnar - I guess you are trying `*current->next = *head;` Change it to `(*current)->next = *head;` Pointer-to-pointer can be tricky and the code becomes hard to read. I avoid them when possible. – Support Ukraine Jul 12 '16 at 13:41

Support Ukraine · Accepted Answer · 2016-07-12T13:44:40.783

@Sourav Ghosh have already explained what was wrong with your code and also suggested one way to solve it. Here is another way.

Instead of passing current and head as variables to be changed inside the function (i.e. as pointer-to-pointer), I would recommend that you use the function return value. In that way you don't have to use pointer-to-pointer.

Something like:

item* add_item(item* head)
{
    // Place a new item in front
    item* current = malloc(sizeof(item));
    current->next = head;
    return current;
}

item* build_list(item* head, FILE *in) {

    char gene[STRLEN];
    char *tok;
    char gene_name[STRLEN];
    char *search = ",";
    int j;

    while (fgets(gene, sizeof(gene), in))
    {
        // Get a new item
        head = add_item(head);

        // Fill data into the new item
        tok = strtok(gene, search);
        strcpy(gene_name, tok);
        for (j = 0; j < 4; ++j)
        {
            tok = strtok(NULL, search);
            head->num[j] = atoi(tok);
        }
    }

    return head;
}

and from maincall it like:

head = NULL;
head = build_list(head, input);

Note: For readability, I skipped all check for failing malloc. In real code you should always check whether malloc returned NULL.

C: Variable is uninitialized for linked list

2 Answers2

Linked