Segment fault in realloc() on loop

Question

I'm trying reallocate more 256 bytes to buffer on each loop call. In this buffer, I will store the buffer obtained from read().

Here is my code:

#define MAX_BUFFER_SIZE 256
//....
int sockfd = socket( ... );

char *buffer;
buffer = malloc( MAX_BUFFER_SIZE );
assert(NULL != buffer);
char *tbuf =  malloc(MAX_BUFFER_SIZE);
char *p = buffer;
int size = MAX_BUFFER_SIZE;

while( read(sockfd, tbuf, MAX_BUFFER_SIZE) > 0 ) {
    while(*tbuf) *p++ = *tbuf++;
    size = size + MAX_BUFFER_SIZE; // it is the right size for it?
    buffer  = realloc(buffer, size);
    assert(NULL != buffer); 
}


printf("%s", buffer);
free(tbuf); 
free(p);
free(buffer);
close(sockfd);

But the above code returns segment fault. Where am I wrong?

while(*tbuf) *p++ = *tbuf++; you are incrementing tBuf, which was malloc(0)d . You will never be able to free it, since you lost the original pointer. Hint: use memcpy(p, tbuf, return_from_read); Also the while (*tbuf) assumes a nul terminated string. — wildplasser
– wildplasser, Commented Mar 21, 2012 at 19:49

David Heffernan · Accepted Answer · 2012-03-21 20:07:48Z

2

These are the problems that are apparent to me:

Your realloc can modify the location to which buffer points. But you fail to modify p accordingly and it is left pointing into the previous buffer. That's clearly an error.
I see potential for another error in that the while loop need not terminate and could run off the end of the buffer. This is the most likely cause of your segmentation fault.
The way you use realloc is wrong. If the call to realloc fails then you can no longer free the original buffer. You should assign the return value of realloc to a temporary variable and check for errors before overwriting the buffer variable.
You should not call free on the pointer p. Since that is meant to point into the block owned by buffer, you call free on buffer alone.

edited Mar 21, 2012 at 20:07

answered Mar 21, 2012 at 19:49

David Heffernan

616k46 gold badges1.1k silver badges1.5k bronze badges

Sign up to request clarification or add additional context in comments.

Comments

cnicutar · Accepted Answer · 2012-03-21 19:48:21Z

2

Thing is read doesn't add a 0-terminator. So your inner while is undoubtedly stepping outside the allocated memory:

while(*tbuf) *p++ = *tbuf++;

Another problem is that you are freeing stuff you didn't receive via malloc. By the time you call free, you will have incremented both p and tbuff which you try to free.

The whole buffer allocation things looks useless as you're not actually using it anywhere.

answered Mar 21, 2012 at 19:48

cnicutar

184k26 gold badges378 silver badges398 bronze badges

Comments

mmirzadeh · Accepted Answer · 2012-03-21 20:04:34Z

2

When you use realloc on buffer, it is possible that the address of buffer is changed as a result of changing the size. Once that happens, p is no longer holding the correct address.

Also towards the end, you are freeing both p and buffer while they point to the same location. You should only free one of them.

edited Mar 21, 2012 at 20:04

answered Mar 21, 2012 at 19:56

mmirzadeh

7,1798 gold badges41 silver badges47 bronze badges

Collectives™ on Stack Overflow

Segment fault in realloc() on loop

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related