Segmentation fault 11 error while reading file into dynamic array

Question

I am trying to read a file that can be empty or can contain numbers into a dynamic array. And I am getting segmentation fault after it reads all the numbers in the file, or the file is empty and I am not sure why, it is doing that.

    //ptr and fp are passed from another function
ptr = malloc(10 * sizeof(int)); 
if (ptr == NULL){
    printf("failed to allocate memory\n");
}
int sz = 10;
int counter = 0;
int fnum;
int *temp;
char lp1[MAXLINE];// maxline is 100
int i = 0;
while(fgets(lp1, MAXLINE, fp)!= NULL){
    if(counter >= sz){
        sz *=2;
        temp = realloc(ptr,sz);
        if (temp == NULL) {
            printf("failed to allocate memory\n");
        }
        if (temp != NULL){
            ptr = temp;
            //free(temp);
        }
    }
    int len = strlen(lp1);
    if (len > 0 && lp1[len-1] == '\n'){
        lp1[len-1] = 0;
    }
    fnum = strToInt(lp1);// number from file
    printf("%i value of lp1 \n", fnum);
    ptr[i] = fnum;
    i++;
    counter++;
}
return sz;

That outcommented `free` call after your reallocation, remove the line completely. — Some programmer dude, May 09 '16 at 00:45
Also, why are you using two variables (`i` and `counter`) that will be the same values? Use one or the other. as both index and the current "size". — Some programmer dude, May 09 '16 at 00:47
//free(temp); is inside the comment , but I removed it and tried running it again and I am still getting segmentation fault. — Gkush, May 09 '16 at 00:53
The commented call have nothing to do with the problem, remember that comments will just be discarded by the compiler. But you will get problem if you remove the comment and have the `free` call, remember that at that point you have two pointers both pointing to the same memory. Freeing one will free that memory, making the other pointer invalid. — Some programmer dude, May 09 '16 at 00:56
`strToInt` might have a bug. Please post a [MCVE](http://stackoverflow.com/help/mcve) — M.M, May 09 '16 at 03:05

Some programmer dude · Answer 1 · 2016-05-09T00:54:43.853

Your first realloc call will shrink the allocated memory from (probably) 40 bytes to 20 bytes. The size provided is in bytes and not the number of "elements". You need to multiply with the base "element" size here too.

Also, if ptr is passed as an argument, then you're doing pass by reference wrong. First of all, C doesn't have pass by reference, only pass by value meaning your function gets a copy of the current value of the variable you pass as argument. This copy is stored in the local variable ptr, and like all other local variables it will go out of scope once the function returns, and all changes to it will be lost.

To fix this you need to emulate pass by value by passing a pointer to the variable, like e.g.

// Code calling your function
...
int *ptr;
you_function(..., &ptr, ...);  // Use address-of operator to pass pointer to variable
...

// Your function
int your_function(..., int **pptr, ...)
{
    ...
    *pptr = malloc(...);  // Use dereference operator to access what pptr points to
    ...
    // Use parenthesis because array indexing have higher precedence than dereference
    (*pptr)[counter++] = (int) strtol(lp1, NULL, 10);
    ...
}

I fixed the realloc to temp = realloc(ptr,sz* sizeof(int)); But I a still getting the error — Gkush, May 09 '16 at 00:54
@Gkush Please read my answer again, especially the added section. And learn how to use a debugger so you can find the crashes yourself, and locate them in your source, and step through the code while monitoring variables and their values. — Some programmer dude, May 09 '16 at 01:11

score 0 · Answer 2 · answered May 09 '16 at 00:59

I believe multiplying by 2 makes the required memory grow exponentially, until it is bigger than the memory your system can allocate. Get the size of the file beforehand and then allocate memory for it. Make sure you release it when you don't need it anymore.

Other thing I recommend you, stop storing numbers on files. Instead create a sqlite database and do it properly. Will save you nightmares and headaches.

Niklas Rosencrantz · Answer 3 · 2016-05-09T07:38:09.140

-1

Here is a simple example that can give you the idea. Have a file and a program, read the file and scan the tokens.

sample.txt

0 1 23 2134 123 12321

123

42

#include <stdio.h>
int main(void) {
    FILE * fp;
    fp = fopen("sample.txt", "r");
    int x;
    while (fscanf(fp, " %d", &x) == 1) { }
    printf("Last integer is %d.\n", x);
    fclose(fp);
}

Output

$ ./a.out 
Last integer is 42.

edited May 09 '16 at 07:38

answered May 09 '16 at 01:08

Niklas Rosencrantz

25,640
75
229
424

Please read [Why is “while ( !feof (file) )” always wrong?](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). This also doesn't answer the question about the crashes. – Some programmer dude May 09 '16 at 01:09
@JoachimPileborg I improved my answer a little, will you have a look again? Didn't know that C had so many mistakes. – Niklas Rosencrantz May 09 '16 at 03:30

Segmentation fault 11 error while reading file into dynamic array

3 Answers3