Segmentation fault with strlen when not ever using strlen?

Question

I have some code which takes a file, reads each line into a new string array (and adds 128 to each character), then assigns each array into an array of pointers, then prints each array. When trying to run the code I get an error stating a segmentation fault due to:

strlen () at ../sysdeps/x86_64/strlen.S:106 106 ../sysdeps/x86_64/strlen.S: No such file or directory.

But I never actually call strlen inside my code?

#include <stdio.h>
#include <assert.h>
#include <stdlib.h>
#define ROW 10
#define COL 40
#define ARGS 2
#define FLIP_VALUE 128

char** read_file (char* argv[], char **array_pointers);
char* new_array (void);
void print_strings (char** array_pointers);

int main(int argc, char* argv[])
{
    char **array_pointers = NULL;
    if (argc == ARGS)
    {
        array_pointers = read_file(&argv[1], array_pointers);
        print_strings(array_pointers);
    }
return 0;
}

char** read_file (char* argv[], char **array_pointers)
{
    FILE* file_name;
    int i = 0, j = 0;
    char c;
    char *temp_array;
    array_pointers = malloc(sizeof(char*) * ROW);
    file_name = fopen(argv[0], "r"); 
    assert(file_name);
    if (file_name) /* if file is not null */
    {
        while (c != EOF) /* while not equal to end of file */
        {
            for (j = 0; j < ROW; j++) /* for each row */
            {
            temp_array = new_array(); /* generate a new array for each new string (row) */
                for (i = 0; i < COL; i++) /* for each char in a row */
                {
                    c = fgetc(file_name);
                    temp_array[i] = c + FLIP_VALUE;
                }
            array_pointers[j] = temp_array; /*assign array pointers to point at each new temp_array */
            }   
        }
    }
    return array_pointers;
}

char* new_array (void)
{
    char* temp;
    temp = malloc(sizeof(char) * COL);
    assert(temp);
    return temp;    
}           

void print_strings (char** array_pointers)
{
    int i = 0;
    for (i = 0; i < COL; i++)
    {
        printf("%s\n",array_pointers[i]);
    }
}

The full stack trace reads as:

#1  0x00007ffff7a84e3c in _IO_puts (str=0x0) at ioputs.c:36
        result = -1
        len = <optimised out>
#2  0x0000000000400806 in print_strings (array_pointers=0x602010)
    at array_of_string_arrays.c:65
        i = 10
#3  0x00000000004006a1 in main (argc=2, argv=0x7fffffffdff8)
    at array_of_string_arrays.c:19
        array_pointers = 0x602010

Suggest you run your program in the debugger and get at least a full stack trace. — kaylum, Dec 18 '15 at 10:38
`c` should be an `int`, otherwise `while (c != EOF)` will never be `false`. You are also creating a new array for each `char`, not for each new string. — mch, Dec 18 '15 at 10:39
It looks like your conditionals in the for loops need to be just `j < ROW` and `i < COL` — user2697817, Dec 18 '15 at 10:39
Your `while (c != EOF)` loop is wrong. See http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong — Andrew Henle, Dec 18 '15 at 10:41
Adding `128` to each `char` in a string and then try to print it with `"%s"` will not print something readable. — mch, Dec 18 '15 at 10:43
@mch the chars inside the file are all negative to begin with, strangely. hence the +128 conversion — Finlandia_C, Dec 18 '15 at 10:45
I wonder, does `fgetc` return a `NUL` char before returning `EOF`? — YSC, Dec 18 '15 at 10:46
Your code is wrong on multiple levels: `c` should be an `int` for `EOF` as already mentioned, you should test `c` for EOF before adding it to the array or you are going to add `EOF` to your array, you should not create an array for each `char` but for each row, you should terminate your array with `\0` or `printf` will probably crash (hence your segfault) and you should loop up to `ROW` not `COL` in `print_strings`. — Holt, Dec 18 '15 at 10:54
[Compile with all warning enabled.](http://justpaste.it/prhr) — pmg, Dec 18 '15 at 11:00
I suggest you write a function that, given a FILE*, returns a single line. Write `main` that calls this function and prints the result. Then do the same in a loop while you get to EOF. Then, *when you test and debug that function and get it working perfectly*, write a function that calls the first one and reads an array of lines. — n. m. could be an AI, Dec 18 '15 at 11:02

score 5 · Answer 1 · answered Dec 18 '15 at 10:55

In print_strings you print the string using printf("%s", str);. This requires the string to be null terminated. The strings you write are not null terminated if you are reading an ordinary text file. You need to add the terminating null byte to the strings you read - or you cannot use printf.

The reason you get a crash is that your printf seems to check first how long the string is (using strlen) before printing it out. strlen increments the char pointer until it reads a null byte. Since you don't have any in the string, strlen reads past the buffer and finally either reads a null byte somewhere in memory or crashes if it is not allowed to read that memory.

score 4 · Accepted Answer · answered Dec 18 '15 at 10:57

There are actually several bugs here:

In read_file, c is initially undefined when first compared to EOF. Do you even need this outer loop?
You should check the result of fgetc as soon as it returns to see if it's EOF.
You are allocating a new temp_array for each character of each row, rather than once for each row. This needs to be moved up into the containing loop.
In print_strings, you are iterating over the rows so you should compare the loop index to ROW, not COL.
When printing a row, you are using %s in a printf. You can't do that, since there is no guarantee that the characters in a row are null-terminated. You could force them to be, by allocating an additional character and storing '\0' in it, or you could include a length field in the format specifier to limit the length (you can use * to pass it as an argument).

I suspect the last bug is the one that's causing the segmentation fault.

score 2 · Answer 3 · edited Dec 18 '15 at 11:10

2

After

temp = malloc(sizeof(char) * COL);

you should

memset(temp,0,sizeof(char) * COL);

because strlen is end by reading 0.

edited Dec 18 '15 at 11:10

Nikolay K

3,770
3
25
37

answered Dec 18 '15 at 10:53

sstask

21
2

Alternatively use `calloc()` or set first (or last) byte allocated to 0. Setting entire array to zero is generally a good simple practice, but frankly it's rarely *needed* – Dima Tisnek Dec 18 '15 at 10:56

Segmentation fault with strlen when not ever using strlen?

3 Answers3