Reputation: 876
I am getting some pretty strange behavior using malloc. I am never allocating more than 4kB, so this seems especially strange to me. My main looks something like:
int main(int argc, char **argv)
{
char *buf;
char *raw = malloc(1024);
fgets(raw, 1024, stdin);
parse(raw, &buf); // Process raw input, get parse length
printf("raw: 0x%p\n", raw); // Outputs 0x00000000
if(raw != NULL)
{ // Only prints when less than 10 characters are entered
free(raw);
printf("raw is not NULL\n");
}
free(buf);
return 0;
}
When I enter less than 10 characters this works okay, when I enter exactly 10 characters I get a segmentation fault, and when I enter more than 10 the output shows that raw is NULL. It should be noted that the size of raw is 1024 malloc'd bytes, so I should have more room to work with.
The parse function is:
int parse(char *src, char **dst)
{
int num_valid = 0, len = strlen(src), j = 0;
// Count number of valid characters
for(int i = 0; i < len; i++)
{
if(src[i] == 'A')
++num_valid;
}
*dst = malloc(num_valid);
for(int i = 0; i < len; i++)
{
if(src[i] == 'A')
*dst[j++] = src[i];
}
// For debugging:
printf("src: 0x%p\n", src); // outputs correct address
return num_valid;
}
This function outputs the correct address, and properly allocates and fills dst. I modified the code here slightly, this is basically a very reduced form of my code. I compiled and ran it (gcc test.c -Werror -Wall) with the same results. It is only after this function returns that my raw pointer becomes NULL, or I get a segfault.
Can someone point me in the right direction? Tell me what exactly I am doing wrong? I've been debugging this little piece of code since yesterday and it is driving me mad.
Upvotes: 0
Views: 111
Reputation: 241701
This doesn't mean what you think it means:
*dst[j++] = src[i];
You meant
(*dst)[j++] = src[i];
What you wrote means:
*(dst[j++]) = src[i];
dst[1]
is whatever happens to follow the variable buf
, so you're using an address from a random memory location and overwriting whatever it might point to; that's undefined behaviour.
As @pm100 points out in a comment, keeping the buffer in a temporary variable is generally better style:
char* buf = malloc(num_valid);
if (!buf) { /* Handle allocation failure */ }
*dst = buf;
/* ... */
/* In loop */
buf[j++] = src[i];
Upvotes: 6