Hacker News Reader

Comment by okasaki | Hacker News Reader

> Immediately clear pointers after freeing

This can be misleading...

    void foo(char *buf) {
        ...
        free(buf); 
        buf = NULL;
    }

    void bar() {
        char *b = malloc(123);
        foo(b);

        // b isn't NULL...
    }

My advice is: always compile and run with AddressSanitizer. Take the time to fix errors/leaks as soon as possible.

View on HN

Why are you freeing b in foo?

It's confusing if you're mallocing in one scope and freeing it in another one (foo is especially confusing, besides being a disaster ready to happen). But if you write bar() like this:

    void bar() {
        char *b = malloc(123);

        foo(b);

        free(b);
        b = NULL;
    }

it looks all right.

Not that I'm advocating this kind of use. I have... mixed feelings about it. But I've seen it used and it doesn't have to be confusing.

Its primary merit is that it makes use-after-free cases immediately obvious, since they're going to trigger a segfault.

But there are better ways to achieve that.

It's just a minimal example.

> Its primary merit is that it makes use-after-free cases immediately obvious, since they're going to trigger a segfault.

But then it hides double free errors.

    int count = 0;

    void foo_destroy(struct foo **f) { free(*f); count--; *f = NULL; }

    void bar() {
        struct foo *f = malloc(sizeof(struct foo));

        foo_destroy(&f); // ok
        foo_destroy(&f); // no error, but count is now wrong
    }

Unless I'm reading your example incorrectly, it's pretty easy to catch those if, in foo_destroy, you ensure that you don't try to free a null pointer.

> It's just a minimal example.

I know -- but it tends to be confusing if you free() something in another scope than the one where you're malloc()-ing it. That's why your example, even if minimal, seemed confusing.

The obvious exception is if you have some reference counting mechanism/GC mechanism in place -- but then the wise thing to do is to not manually touch pointers all over the code.

If `foo` doesn't own `buf`, it should be declared `char * const` (or `char const * const`). If `foo` takes owner ship of `buf` (and is intended to free it), it should take a pointer to the pointer to clear it for the caller. This is one way to write it:

    void foo(char **const bufptr) {
        ...
        free(*bufptr); *bufptr = NULL;
    }

You can dereference `bufptr` at the top of the function to make the code more readable. This is how (almost) every object freeing function works in my project.

Valgrind / asan / clang-analyzer on their own take care of most of the issues listed under memory management.

I agree to use these heavily; but be careful as they are not a silver bullet. There are conditions they can't and won't catch.

Reply on news.ycombinator.com