free a pointer and have it point to NULL

Discussion in 'C Programming' started by G G, Apr 22, 2014.

  1. G G

    G G Guest

    memory allocated on the heap once freed should the pointer be assigned NULL?
     
    G G, Apr 22, 2014
    #1
    1. Advertisements

  2. G G

    Stefan Ram Guest

    There is no general answer possible.

    It is not necessary, but might be used to follow
    project-specific guidelines for memory management
    or as »defensive programming«.

    To get a more specific answer, post an SSCCE.
     
    Stefan Ram, Apr 22, 2014
    #2
    1. Advertisements

  3. G G

    James Kuyper Guest

    Only if there's a danger of the pointer being used again after being
    free()'d. Even then, it's only useful if other code, elsewhere, checks
    if(p) before trying to dereference p. Finally, if there are multiple
    saved pointers, all pointing into the same block of allocated memory,
    setting one of those pointers to be null won't help avoid problems with
    any of the other pointers. They must all be protected by if(p).

    I often use the following approach, which removes all need for nulling
    out the free()'d pointer:

    {
    int *p = malloc(num_elements*sizeof *p);
    if(p==NULL)
    {
    // Error handling
    }
    else
    {
    // Code which uses *p
    free(p);
    }
    // Key point: *p is not used anywhere in this section
    }

    Since the lifetime of 'p' ends immediately after free(p), there's no
    need to write "p = NULL;". This approach cannot be used if the malloc()
    and free() must occur in different functions, or at very different
    levels within the same function.
     
    James Kuyper, Apr 22, 2014
    #3
  4. G G

    G G Guest

    On Tuesday, April 22, 2014 5:22:34 PM UTC-4, G G wrote:

    ok.

    thanks Stefan and James
     
    G G, Apr 22, 2014
    #4
  5. Maybe.

    After free(ptr), the value of ptr is *indeterminate*, meaning that any
    attempt to refer to its value has undefined behavior. In practice, just
    reading the pointer probably isn't going to do anything harmful. But
    having such a pointer value sitting around creates a potential source of
    bugs, and setting it to NULL can remove *some* such bugs, or at least
    change them into different bugs that might be easier to detect.

    On the other hand, if the pointer object is just about to go out of
    scope and therefore cease to exist, there's no point in setting it to
    NULL. Example:

    {
    unsigned char *buffer = malloc(some_size);
    /* error checking omitted */
    /* code that uses the pointer omitted */
    free(buffer);
    }

    Setting buffer to NULL after calling free() would be pointless, because
    there's no way to refer to its value accidentally.
     
    Keith Thompson, Apr 22, 2014
    #5
  6. It can't hurt since any attempt to use the now invalid pointer
    again would lead to an immediate crash of the program. But then
    it depends a bit on circumstances: if the free() is at the end
    of a function and there's no chance that the pointer is acci-
    dentally used again, then there's not much good it will do.
    And, of course, each free() will have to be followed by
    another line of code, which can become tedious and make
    things a bit harder to read - unless you use a macro like

    #define safe_free( addr ) \
    do { \
    free( addr ); \
    addr = NULL; \
    } while ( 1 == 0 )

    instead of free() everywhere. I've done that in several cases
    where I felt the need to be paranoid;-)

    Regards, Jens
     
    Jens Thoms Toerring, Apr 22, 2014
    #6
  7. "do { ... } while (0)" is the usual idiom for writing a macro that can
    be used in a context requiring a statement; the "1 == 0" just seems
    (mildly) obfuscated.

    But when possible, it's usually better to write a macro that can be used
    wherever an expression can appear. In this case:

    #define safe_free(addr) ( free(addr), (addr) = NULL )
     
    Keith Thompson, Apr 22, 2014
    #7
  8. G G

    Kaz Kylheku Guest

    Once you free a pointer, all variables and other storage locations which previously
    held that pointer's value now hold an indeterminate value which must not be used.

    So, you don't lose any valuable information by overwriting some of the copies
    of that value, or all of them, with null. On the contrary, you are scrubbing
    away dangerous information.

    If you do this in an existing program that appears to work, it may now crash with
    a null pointer dereference. This indicates that it was relying on the
    indeterminate value: it has a use-after-free bug which was revealed by changing
    the pointer to null.

    In some programs, you must set certain pointers to null after freeing, because
    parts of the program specificaly check for the value and rely on it as an
    indication "the usual object is not there". Should you design everything with
    such checks? The answer is no: there are situations in which an object is just
    expected to be there for the entire lifetime of another object, and so a null
    check doesn't make sense.

    So:

    - Overwriting stale pointers with nulls is a good idea for turning
    unpredictable behavior (use of garbage poitners) into null pointer
    dereferences (a condition that, on many platforms, produces some predictable
    exception which can be used to pinpoint the location and debug the program).
    But watch out: retroactively doing this in a program can suddenly reveal
    bugs, and you are on the hook for them because it looks like your change
    "broke" the program. So you should do this when you're ready to take
    responsibility to follow up and fix the null pointer dereferences that
    may be revealed.

    - Whether you use the null pointer as a special "not there value" and actually
    check for it in key places in the code is debatable. It hinges on whether
    it is reasonable for an object which is reachable through the pointer to
    to sometimes not be available. If this is NOT reasonable (i.e. the pointer
    should always be non-null and valid, except perhaps in very eearly
    initialization stages of the code, or late in the finalization), tne you will
    only confuse yourself by adding checks for null. When you add these checks,
    then someone who reads the code (like yourself, six weeks later) begins to
    suspect that it is expected behavior and fine for the pointer to sometimes be
    null. And that makes the code look more complicated than it is. People then
    waste time adding more such code, thinking it is necessary, testing it, and
    looking for bugs which cannot exist.
     
    Kaz Kylheku, Apr 22, 2014
    #8
  9. G G

    Joe Pfeiffer Guest

    Unfortunately, this doesn't help with situations like

    a = malloc(size);
    b = a;
    safe_free(a);

    so it's always struck me as a giving a false sense of security.
     
    Joe Pfeiffer, Apr 22, 2014
    #9
  10. G G

    Kaz Kylheku Guest

    it helps in situations like

    safe_free (proxy->obj); /* proxy is the only gateway to obj */

    In situations where you you deal with the shared reference problem, perhaps
    with reference counting, it also helps. Even if proxy is not the only way to
    reach obj, it makes sense to do:

    drop_ref (proxy->obj); /* drop_ref is a macro which also sets obj to null */

    proxy may well have had the last reference to obj. We have no right to access
    obj through proxy, even if obj still exists. Since we have no right to access
    obj through proxy, it's best to null out the pointer, so that any attempt to do
    so is flagged as a null pointer dereference.

    In other words, the management of multiple copies is a separate, problem; it
    doesn't invalidate the setting of just one copy to null, which can still a
    valuable approach when we have somehow addressed the management of sharing.
     
    Kaz Kylheku, Apr 22, 2014
    #10
  11. G G

    Joe Pfeiffer Guest

    I don't think it's entirely separate -- when I see someone suggest
    zeroing freed pointers in isolation, my impression is that they're
    expecting that, in and of itself, to be helpful in spotting use of freed
    memory. Without integrating into a shared reference management system,
    I don't see it as useful.
     
    Joe Pfeiffer, Apr 23, 2014
    #11
  12. This is simply not true, relying on the assumption will end in pain (or
    at least subtle bugs). I come from an x86 OS development background
    where the problem is particularly prevalent.

    The set of systems where the value 0 is actually an invalid pointer is
    increasingly small. All x86 systems as well as most ARM and MIPS
    systems can be up so the value 0 is a genuine pointer[1]. Under the
    POSIX standard by which UNIX as well as Linux and the BSDs abide,
    functions such as mmap()[2] can be used to put valid memory under a
    pointer whose value is 0.

    From an OS point of view, accidentally dereferencing a NULL pointer is
    particularity bad as, while it will tend to result in a crash, malicious
    userspace can use the bug to its advantage.


    I wish to add that I am not advocating avoiding the use of NULL. I
    would certainly encourage people to explicitly invalidate their pointers
    if the pointers are not immediately going out of scope[3]. However,
    don't fall into the trap of assuming that use of a NULL pointer will
    result in a program crash.

    It is unfortunate that implementations of 64bit x86 code don't use
    0x8000000000000000 (or something similar) as the machine representation
    of a NULL pointer. For the foreseeable future, 62/64ths of the virtual
    address space will unconditionally generate a processor exception
    irrespective of other state. I suspect this was down to inertia more
    than the fact that most people can't write portable C.

    ~Andrew



    [1] In 32bit mode without paging, the only way to get at the system IVT
    is to create a pointer to the value 0 and use it.

    [2] OpenBSD takes the precaution of actively preventing mapping memory
    regions to 0 or 0x00007ffffffff000. This in turned out to be a
    VeryGoodIdea from a security point of view as it protected them against
    CVE-2012-0217.

    [3] Any static analyser worth its salt will pick up accidental reuse of
    a NULL pointer. And you are programming with the aid of a static
    analyser aren't you...
     
    Andrew Cooper, Apr 23, 2014
    #12
  13. G G

    Kaz Kylheku Guest

    You make some good points, but:

    * The null pointer is what the C language gives us, however it is defined
    by the implementation. It is C's special pointer value which can be
    accessed, but not displaced or dereferenced (with any defined
    results, anyway), and which doesn't point to any object.

    * If you use some techniques like mmap to put an object at the address
    that is implied by the null pointer, you're sticking a fork in the
    toaster.

    * Compilers and debugging tools can check for null pointers without
    assistance from memory management hardware. I seem to remember that some
    popular compilers for the MS-DOS environment had a code generation option for
    this. If my life depended on it, I could patch gcc into doing it.
    Also, I think that the Valgrind debugger on Linux will still report null
    pointer dereferences even if the program uses mmap to make the zero page
    valid. So really, it's not a smart thing to do. In Linux kernel mode, null
    pointers are trapped also; the kernel painstakingly arranges this for itself.

    * MMU null pointer checks are not perfect, indeed; that is valid point.
    For instance if there is only a 4096 byte unmapped page at the
    zero address, x is null, but the expression x->memb is such that the offset
    of memb is greater than 4096, then the expression can evaluate
    without triggering an exception. Just because a measure doesn't always
    work doesn't mean it's not worthwhile. Soap doesn't remove all stains;
    we still wash mostly with soap.

    * It is valid for a compiler to define 0x8000000000000000 as the null pointer.
    Unfortunately, it will break lots of nonportable code which assumes that
    pointers located in memory which comes from calloc, or which was memset to
    zero, are implicitly initialized to null (and can be tested as being equal to
    null). We can jump up and down and fume about such code being nonportable;
    doing so won't make it go away.
     
    Kaz Kylheku, Apr 23, 2014
    #13
  14. (snip, someone wrote)
    It is probably better than leaving the previous value, though,
    as it is likely that the previous data is still there for a little
    while.
    On protected mode x86, from the 80286 on, segment selector zero is
    a special null selector. The value in a segment register has to be
    either a valid selector or zero. But that only matters when using
    more than one segment, which is rare to nonexistent.

    (Last I knew, the Watcom compilers could generate large mode 32 bit
    code, and possibly OS/2 could run it.)
    I once had to debug a program, written by someone else, that would
    get into an infinite loop. It turned out that it was trying to free
    a linked list that either wasn't allocated or had been previously
    deallocated. It used the usual recursive delete algorithm, but the
    actual list, after over 100 links, had a loop in it. Somehow
    hundreds of pointers were actually pointing to other pointers,
    instead of some invalid address.

    (snip)

    -- glen
     
    glen herrmannsfeldt, Apr 23, 2014
    #14
  15. G G

    Öö Tiib Guest

    I have seen even 'while (__LINE__ == -1)' there to silence a warning
    of particular compiler that sometimes complained about condition
    being constant expression (like on case of '0' or '1 == 0' but not on
    case of '__LINE__ == -1').
     
    Öö Tiib, Apr 24, 2014
    #15
  16. G G

    James Kuyper Guest

    On 04/24/2014 03:38 AM, �� Tiib wrote:
    ....
    __LINE__ is replaced by the actual line number in translation phase 4;
    from that point on, if the line number happens to be 54, that construct
    is indistinguishable from 54 == -1. I wonder how that compiler made its
    decision not to warn about that construct, while warning about 1==0?
     
    James Kuyper, Apr 24, 2014
    #16
  17. G G

    Joe Pfeiffer Guest

    Just came across this interesting story about openssl and the new
    libressl project:

    http://arstechnica.com/information-...eyond-repair-claims-creator-of-libressl-fork/

    Following the links from the story sends one to a changelog with
    possibly the densest collection of coding WTFs I've ever seen:

    http://freshbsd.org/search?project=openbsd&q=file.name:libssl
    (and I'd argue that these are enough plain-bad-C WTFs as opposed to
    system-specific WTFs that it's on topic here).

    There's even a "greatest hits" blog:

    http://opensslrampage.org/post/82973125757/strncpy-dest-src-strlen-src-is-safe-right

    I note (and here's why I put it in this thread) that the libressl team
    disagrees with me on the utility of nulling freed pointers.
     
    Joe Pfeiffer, Apr 24, 2014
    #17
  18. G G

    Ian Collins Guest

    #include <stdlib.h>

    struct fifo;

    struct fifo *fifo_init(struct fifo *, void *, size_t);

    #define fifo_init3(...) (fifo_init)(__VA_ARGS__)
    #define fifo_init1(fifo) fifo_init3((fifo), (void *)0, 0U)
    #define fifo_init(...) XPASTE(fifo_init, NARG(__VA_ARGS__))(__VA_ARGS__)

    void f( struct fifo* ff )
    {
    fifo_init3( ff, NULL, 0 );
    fifo_init1( ff );
    }
     
    Ian Collins, Apr 25, 2014
    #18
  19. G G

    Ian Collins Guest

    The warning makes sense, see:

    # gcc -std=c99 -Wall -pedantic ~/temp/m.c -c
    /home/ian/temp/m.c: In function 'f':
    /home/ian/temp/m.c:18:21: warning: ISO C99 requires rest arguments to be
    used

    # gcc -std=c99 -Wall -pedantic ~/temp/m.c -E
    ....
    void f( struct fifo* ff ) {
    /home/ian/temp/m.c:18:21: warning: ISO C99 requires rest arguments to be
    used
    (fifo_init)((ff), (void *)0, 0U);
    }

    # c99 ~/temp/m.c -E
    void f( struct fifo* ff ) {
    (fifo_init)((ff), (void *)0, 0U);
     
    Ian Collins, Apr 25, 2014
    #19
  20. G G

    Öö Tiib Guest

    I do not know but I can try to hypothesise. Compiler vendors have
    always put (increasing over time) effort into static analysis and
    diagnostics of code. Sometimes warnings are amazingly clever,
    sometimes just annoying. Some are byproducts of optimisation.
    When compiler can optimise it out then why it is written? May be
    it deserves warning? That can be case with constant condition.

    It is possible that phase 4 "sets a flag" of the value 54 being "variable"
    enough and so not deserving warnings when compiler optimises the
    whole branch, switch or loop out that uses it as condition.
     
    Öö Tiib, Apr 27, 2014
    #20
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.