CODEWITHSUNDEEP
cstringmemory-leaksvalgrind
hat
hat

Reputation: 860

Valgrind reports SIGSEGV and use of uninitialised value

Edit: All valgrind errors where solved by initialising i.

I am working with strings; reading strings from a file, separating them using strsep(), converting them to int using atol(). My code compiles on gcc and runs without errors, but when checked with valgrind I get the error mentioned in the title.

I am trying populate a two-dimensional array grid[20][20] with int values from a text file. The text file contains lines of numbers separated by spaces, something like 20 81 65 07 54 72 13 28 66 95 00 20 00 84 06 30 85 43 15 73\n

My code, (excluding all of the file IO) is as below. (Note that I do handle the file correctly: checking the result of fopen, closing with fclose).

Updated code

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define STR_LEN     20
#define NUM_LINES   20
#define MAX_SIZE    100

int main(void){
    char input_str[MAX_SIZE];   //string to hold lines read from file
    char * str_array[STR_LEN];  //array of strings to split the lines into
    int grid[20][20];
    char * token, *str, *tofree;
    int line;
    for (line = 0; line < NUM_LINES; ++line){   //for every line in the file 
        
        if (fgets(input_str, MAX_SIZE, fp)!=NULL){  //read the file's line into input_str
            tofree = str = strdup(input_str);   //set str to copy of input_string, and tofree to point to beginning of str's memory, to free later
            
            int i = 0;
            while ((token = strsep(&str, " "))){    //split str on " ", keeping everything in between in token
                str_array[i] = token;   //add token to str_array
                ++i;
            }
            for (i = 0; i < STR_LEN; ++i){
                grid[line][i] = atol(str_array[i]); //convert strings to int and store in num_array     
                printf("grid[%d][%d]: %d\n", line, i, grid[line][i]);

            }
            free(tofree);   //free str by freeing tofree, as tofree points to str's memory
        }
    }
    printf("%d", grid[0][0]);
    return 0;
}

I compile this code with gcc -Wall -g -c myfile.c. It both compiles and runs fine, with no errors. Checking the values using printf() just after the values have been passed to grid shows all correct values. However, the last printf() in the code shows that grid[0][0] contains junk, even though it was correct before. In fact, all elements in grid[0] have been corrupted.

Running valgrind with valgrind --leak-check=full --track-origins=yes --dsymutil=yes --show-leak-kinds=all ./productGrid.elf gives me

$ valgrind --leak-check=full --track-origins=yes --dsymutil=yes --show-leak-kinds=all ./productGrid.elf

==2252== Command: ./productGrid.elf
==2252== 
==2252== Use of uninitialised value of size 4
==2252==    at 0x106EC: main (in /home/jesse/C/Proj. Euler/productGrid.elf)
==2252==  Uninitialised value was created by a stack allocation
==2252==    at 0x10656: main (in /home/jesse/C/Proj. Euler/productGrid.elf)
==2252== 
==2252== Invalid write of size 4
==2252==    at 0x106EC: main (in /home/jesse/C/Proj. Euler/productGrid.elf)
==2252==  Address 0x7e2b6c38 is not stack'd, malloc'd or (recently) free'd
==2252== 
==2252== 
==2252== Process terminating with default action of signal 11 (SIGSEGV)
==2252==  Access not within mapped region at address 0x7E2B6C38
==2252==    at 0x106EC: main (in /home/jesse/C/Proj. Euler/productGrid.elf)
==2252==  If you believe this happened as a result of a stack
==2252==  overflow in your program's main thread (unlikely but
==2252==  possible), you can try to increase the size of the
==2252==  main thread stack using the --main-stacksize= flag.
==2252==  The main thread stack size used in this run was 8388608.
==2252== 
==2252== HEAP SUMMARY:
==2252==     in use at exit: 413 bytes in 2 blocks
==2252==   total heap usage: 3 allocs, 1 frees, 4,509 bytes allocated
==2252== 
==2252== 61 bytes in 1 blocks are still reachable in loss record 1 of 2
==2252==    at 0x483E380: malloc (vg_replace_malloc.c:299)
==2252==    by 0x48E188B: strdup (strdup.c:42)
==2252==    by 0x106C9: main (in /home/jesse/C/Proj. Euler/productGrid.elf)
==2252== 
==2252== 352 bytes in 1 blocks are still reachable in loss record 2 of 2
==2252==    at 0x483E380: malloc (vg_replace_malloc.c:299)
==2252==    by 0x48D11F3: __fopen_internal (iofopen.c:69)
==2252==    by 0x10681: main (in /home/jesse/C/Proj. Euler/productGrid.elf)
==2252== 
==2252== LEAK SUMMARY:
==2252==    definitely lost: 0 bytes in 0 blocks
==2252==    indirectly lost: 0 bytes in 0 blocks
==2252==      possibly lost: 0 bytes in 0 blocks
==2252==    still reachable: 413 bytes in 2 blocks
==2252==         suppressed: 0 bytes in 0 blocks
==2252== 
==2252== For counts of detected and suppressed errors, rerun with: -v
==2252== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 6 from 3)
Segmentation fault

Any ideas on what I'm doing wrong?

Upvotes: 0

Views: 161

Answers (3)

Alex G
Alex G

Reputation: 1917

You should initialize i at the beginning.

Also about this piece of code:

while ((token = strsep(&str, " "))){
    str_array[i] = token;   //add token to str_array
    ++i;
 }

for (i = 0; i < STR_LEN; ++i){
     grid[line][i] = atol(str_array[i]);
     printf("grid[%d][%d]: %d\n", line, i, grid[line][i]);
}

why are you counting the i if you are going to zero it below? Maybe you should use a different counter and loop until this and not STR_LEN.

Hope this helps.

Upvotes: 1

Mance Rayder
Mance Rayder

Reputation: 363

I can't comment, so I post a new answer. As @alk said, i it's uninitialized and if you just initialize it at the definition, the while won't work. Use a for(;;), as you need the assignment.

Update your code if you are still having issues.

Upvotes: 1

alk
alk

Reputation: 70971

i is used uninitialised here: 

  str_array[i] = token; 
  ++i;

Note:

If you compile with symbols (option -g to GCC) then Valgrind annotates its logging with references to the source lines.

Upvotes: 2

Related Questions