Pointers to pointers in C

Question

I'm trying to understand how pointers to pointers work and I came out with this example, and it compiles fine. But, when it is executed, I get a segmentation fault. PS: I don't want f1() to return char *.

#include <stdio.h>
#include <stdlib.h>
#include <assert.h>

int f1(char **str_);

int main(int argc, char **argv)
{
    char *str = NULL;

    f1(&str);     
    printf("str : %s\n", *str);

    str = realloc(str, (size_t) 0);   
    assert(str == NULL);

    return 0;
}

int f1(char **str_)
{
    if ((*str_ = realloc(*str_, sizeof(char) * 12)) == NULL)
    {
        fprintf(stderr,"realloc() failed\n");
        exit(3);
    }

    (*str_) = "hello there";

    return 0;
}

• What is wrong with the code?

Answer 1

You either need to turn on more warnings in your compiler or get a better compiler. When I compile your code, I get the warnings:

mem.c: In function ‘main’:
mem.c:12: warning: format ‘%s’ expects type ‘char *’, but argument 2 has type ‘int’
mem.c:12: warning: format ‘%s’ expects type ‘char *’, but argument 2 has type ‘int’
mem.c: At top level:
mem.c:7: warning: unused parameter ‘argc’
mem.c:7: warning: unused parameter ‘argv’

I'm not quite sure why the warning on line 12 is repeated, but both GCC 4.2 and 4.6.1 on MacOS X 10.7 give it twice. The warnings about argc and argv are a good reason for using int main(void) when you are not using the command line arguments; they aren't a big problem.

The warning (really, an error) about %s and int vs char * is sufficient to account for your crash - it is not, however, the only issue with the code. In fact, there is also:

1. an incipient memory leak,
2. an actual leak,
3. a place where you are (accidentally) relying on undefined behaviour, and
4. a place where you're relying on implementation-defined behaviour which may not match your expectations.

Your code annotated:

#include <stdio.h>
#include <stdlib.h>
#include <assert.h>

int f1(char **str_);

int main(int argc, char **argv)
{
    char *str = NULL;

    f1(&str);     
    printf("str : %s\n", *str);

    str = realloc(str, (size_t) 0);   /* (3) Undefined behaviour */
    assert(str == NULL);              /* (4) Implementation-defined behaviour */

    return 0;
}

int f1(char **str_)
{
    if ((*str_ = realloc(*str_, sizeof(char) * 12)) == NULL) /* (1) Incipient leak */
    {
        fprintf(stderr,"realloc() failed\n");
        exit(3);
    }

    (*str_) = "hello there";  /* (2) Actual leak */

    return 0;
}

Discussing these in the numbered sequence:

1. The incipient leak arises if the memory reallocation fails. *str is the only place where the previous value of the pointer to the allocated memory is stored, and if realloc() fails, it returns 0 (null pointer) but does not release the old memory, but you no longer have a pointer to the old memory.

   The fix:

   char *new_mem = realloc(*str, 12);
   if (new_mem == 0)
       ...error handling...
   *str = new_mem;
   

   Rule of thumb: do not assign the return value from realloc() to the variable that is its first argument.

2. The actual leak arises because you assign the pointer to a string constant over the pointer to the newly allocated memory. The simplest fix is to use strcpy(), though you need to add #include <string.h> when you do so. You would also, normally, make sure you allocate just enough space for the string you are going to copy, leading to:

   char const hello[] = "hello there";
   char *new_mem = realloc(str, sizeof(hello));
   //char *new_mem = realloc(str, strlen(hello)+1);
   if (new_mem == 0)
       ...handle error...
   *str = new_mem;
   strcpy(new_mem, hello);
   

   In this example, I can use sizeof(hello) because the size of a string includes the null terminator, and because the actual array definition is in scope. If the string to be copied was passed into the function as a pointer, then the alternative using strlen(hello)+1 is correct (and using sizeof() is incorrect), even though it requires a runtime computation of the length instead of a compile time computation as shown.

3. The undefined behaviour arises because of the memory leak at (2). You try to realloc() a string constant, not the pointer returned by realloc(). This leads to undefined behaviour; it could crash (because realloc() tries to write control information into read-only memory) or it might simply mess up the memory system, leading to a crash some time later.

   The fix for this is the correction for item (2).

4. The implementation-defined behaviour arises because the C standard says:

   §7.20.3 Memory management functions ¶1: [...] If the size of the space requested is zero, the behavior is implementation defined: either a null pointer is returned, or the behavior is as if the size were some nonzero value, except that the returned pointer shall not be used to access an object.

   You are asserting that your implementation will choose the first option, but it might choose the second. The fix is to remove the unwarranted assertion.

So, that's a total of 5 problems in the code, only one of which a compiler is likely to help you with.

Answer 2

The first line of main() sets the variable str to NULL and passes a pointer to it to f1.

f1 runs perfectly fine. The outcome of f1 is that the variable str inside of main is now a pointer to a space in memory holding the string (literal) "hello there".

Your next line, the printf, segfaults. Why? Because you are trying to print *str (note the asterisk here!!) as a string (format specifier %s). What is *str when iterpreted as a string? Whatever "address" is denoted by "hell" probably (at least on a 32-bit machine). Doubtful that address is in your process space.

Classic segfault.

Try passing str rather than *str to printf.

That will work, see http://codepad.org/Mh00txen

There are other issues with the code, e.g. the realloc of 12 chars in f1 does nothing except cause a memory leak because you immediately reassign that pointer to point to a string literal, but that is not the cause of your segfault.