3 ms·

> Immediately clear pointers after freeing This can be misleading... void foo(char *buf) { ... free(buf); buf = NULL; }

by okasaki 11y ago

> 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.

viraptor 11y ago
Valgrind / asan / clang-analyzer on their own take care of most of the issues listed under memory management.
- albinofrenchy 11y ago
  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.
weland 11y ago
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.
- okasaki 11y ago
  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 }
  - weland 11y ago
    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.
btrask 11y ago
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.

by okasaki 11y ago

viraptor 11y ago

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

albinofrenchy 11y ago
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.

weland 11y ago

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.

okasaki 11y ago
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 }
- weland 11y ago
  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.

btrask 11y ago

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.