segmentation fault when reading from file to buffer

Question

I need to read a file and then send it through a socket, but fread crash for some reason. Any idea?

//Get file length
fseek(fd, 0, SEEK_END);
int fileLen=ftell(fd);
fseek(fd, 0, SEEK_SET);

//Allocate memory
buf=(char *)malloc(fileLen+1)* sizeof(char));
if (!buf)
{
    fprintf(stderr, "Memory error!");
}
rewind(fd);

fread((void *)buf, strlen(buf)+1, 1, fd);

You get the 'Notwork' badge: str*() in network code. Hint: what do you think strlen() actually does? — Martin James, Apr 09 '14 at 08:08
Also can you not get error for it `buf=(char *)malloc(fileLen+1)* sizeof(char));`? because missing `)`. — Jayesh Bhoi, Apr 09 '14 at 08:14

score 3 · Accepted Answer · edited May 23 '17 at 12:07

There are quite a few basic confusions in that code.

ftell() returns long, not int. Also it should be const, since the file's size is assumed not to change while we read it.
There's no need to call rewind() and use fseek() to offset 0.
Don't cast the return value of malloc() in C.
You are not dealing with strings, so don't add one for some "terminator".
Don't scale allocations by sizeof (char), that's always 1.
Check that the allocation succeeds before relying on the result.
Don't cast the buffer pointer to void * in fread(), that's completely pointless.
Use the file length in the fread(), calling strlen() on an undefined pointer is undefined behavior.
Verify that the fread() succeeds.

Thanks! I was just wondering what the deeper sense of all this was... `fread (&buf, ... )` would have been perfectly sufficient, wouldn't it? — syntaxerror, May 11 '15 at 21:14

score 1 · Answer 2 · answered Apr 09 '14 at 08:08

1

fread((void *)buf, strlen(buf)+1, 1, fd);

should be

fread((void *)buf, 1, fileLen, fd);

You did not initialized the contents of buf, so strlen(buf) will not return the correct length of buf.

By the way, buf=(char *)malloc(fileLen+1)* sizeof(char)); should be buf=(char *)malloc(fileLen);, there is no need to allocate that extra byte, and sizeof(char) always returns 1.

answered Apr 09 '14 at 08:08

Lee Duhem

14,695
3
29
47

Heh - missed that '*'. As soon as I see strlen in network code, I'm blind to anything else. – Martin James Apr 09 '14 at 08:11
Actually strlen return the right dimension of the buffer, i've checked using a printf. I also tried your solution but it did not work – NLK511 Apr 09 '14 at 08:13
printf("\nbufSize = %d", strlen(buf)); it's printing the number of byte of the opened file. I tried to use sizeof and it goes segmentation fault in the printf – NLK511 Apr 09 '14 at 08:24
@NLK511 can post more source? – Jayesh Bhoi Apr 09 '14 at 08:39
Also, how big is the file? – Martin James Apr 09 '14 at 08:40
@NLK511 You already knew the length of `buf`, it is `fileLen+1`, you used it in `malloc()` to allocate that `buf`, so why do you try to figure it out wrongly by `strlen(buf)`? – Lee Duhem Apr 09 '14 at 09:37

segmentation fault when reading from file to buffer

2 Answers2