Jacob H
Jacob H

Reputation: 876

malloc'd pointer resetting itself to NULL?

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

Answers (1)

rici
rici

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

Related Questions