CODEWITHSUNDEEP
cmemory-managementrealloc
Jack
Jack

Reputation: 16724

Segment fault in realloc() on loop

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?

Upvotes: 0

Views: 1125

Answers (3)

David Heffernan
David Heffernan

Reputation: 612804

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.

Upvotes: 2

mmirzadeh
mmirzadeh

Reputation: 7079

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.

Upvotes: 2

cnicutar
cnicutar

Reputation: 182609

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.

Upvotes: 2

Related Questions