CODEWITHSUNDEEP
cvalgrind
Austin Johnston
Austin Johnston

Reputation: 219

C: Valgrind: Use of uninitialized value of size 4 AND Use of uninitialised value of size 4

These 2 errors are coming up with a linked list/low level file read/strtok program.

==10982== Use of uninitialised value of size 4
==10982==    at 0x40C3899: strtok (strtok.S:197)
==10982==    by 0x8048719: main (main.c:9)
==10982==  Uninitialised value was created by a stack allocation
==10982==    at 0x80487D2: buildList (functions.c:13)
==10982== 
==10982== Use of uninitialised value of size 4
==10982==    at 0x40C38C1: strtok (strtok.S:223)
==10982==    by 0x8048719: main (main.c:9)
==10982==  Uninitialised value was created by a stack allocation
==10982==    at 0x80487D2: buildList (functions.c:13)
==10982== 
==10982== Conditional jump or move depends on uninitialised value(s)
==10982==    at 0x40C38C4: strtok (strtok.S:224)
==10982==    by 0x8048719: main (main.c:9)
==10982==  Uninitialised value was created by a stack allocation
==10982==    at 0x80487D2: buildList (functions.c:13)

Here is the function buildList

void buildList(int fdin, int lines)
{
    char ara[4096];
    int num;
    double p;
    read(fdin, ara, 4096);
    char *nl = "\n";
    char *ws = " ";
    char *temp =  strtok(ara, " ");
    while(temp != NULL)
    {
        Stock *s = (Stock*)calloc(1, sizeof(Stock));
        s->c.symbol = (char*)calloc(strlen(temp)+1, sizeof(char));
        strcpy(s->c.symbol, temp);
        temp = strtok(NULL, "\n");
        s->c.name = (char*)calloc(strlen(temp)+1, sizeof(char));
        strcpy(s->c.name, temp);
        temp = strtok(NULL, "\n");
        sscanf(temp, "%lf", &p);
        temp = strtok(NULL, "\n");
        s->price = p;
        sscanf(temp, "%d", &num);
        s->shares = num;
        Node *n = (Node*)calloc(1, sizeof(Node));
        n->data = s;
        addOrdered(n);
        temp = strtok(NULL, " ");
    }
    close(fdin);
}

I cannot figure out why this error occurs. From what I've read it is because I am assigning things to a char* from strtok without assigning any memory to them. However this is how I have done it in the past and I think it has been fine.

Upvotes: 6

Views: 8698

Answers (3)

jxh
jxh

Reputation: 70492

As others have surmised, valgrind is complaining that strtok is making use of uninitialized memory. The question then turns to why, since it seems like it is being initialized by:

read(fdin, ara, 4096);

However, read() does not \0 terminate the data, while strtok expects a \0 terminated string. Make sure the input is terminated properly:

ssize_t result = read(fdin, ara, sizeof(ara)-1); /* leave room for '\0' */
ara[result > 0 ? result : 0] = '\0';

The memset(ara, 0, sizeof(ara)); suggestion by Lee Daniel Crocker amounts to the same kind of fix, but it only works if the file contains less than sizeof(ara) bytes of input. You are still exposed to a similar problem if the file you read from has 4096 bytes of data or more. Then, strtok will be forced to scan past the end of the buffer to look for the \0, leading to undefined behavior.

Upvotes: 6

user862787
user862787

Reputation:

read(fdin, ara, 4096);

...which could return:

  • -1, meaning the read got an error, and ara wasn't filled in;
  • 0, meaning you hit the end of the file, and ara wasn't filled in;
  • a value < 4096, meaning there are fewer than 4096 bytes left in the file, and not all of the bytes of ara are filled in.

Always check the return value of read().

char *temp = strtok(ara, " ");

$ man strtok

...

The strtok() function is used to isolate sequential tokens in a null-ter-
minated string, str. ...

There is nothing whatsoever that guarantees that ara has a null character ('\0') in it, so you cannot validly hand it to strtok(). Make it char ara[4097] and do ara[4096] = '\0'; after the read if you want to make sure that there's a '\0' in it.

Or, even better, if read()'s return value is in a variable data_read, and you've already checked to make sure it's not -1, do ara[data_read] = '\0';, so that you set the byte after the last one read to '\0'.

And what happens if the file has more than 4096 characters, and your read() call reads in, for example, several lines and the beginning of another line? Given that, you might want to consider using fopen() to open the file and use fgets() to read lines.

Upvotes: 3

Imre Kerr
Imre Kerr

Reputation: 2428

Does the data read by read have a null terminator at the end? If not, that's probably the issue.

Upvotes: 1

Related Questions