Hacker News Reader

Comment by codemusings | Hacker News Reader

> The pattern "x = realloc(x, ...)" is a bad idea, it leaks the original memory on failure. Since the code seems to car about its memory management (and since it's a tutorial), I think this is worth mentioning (and fixing).

What would the alternative be here? Allocate a new region, copy everything over and free the other region? Or is there something better?

> I generally recommend against scaling string allocations by sizeof (char); that's always 1 so it doesn't do anything. Some people seem to feel that it's part of a general usage pattern that makes the intent more clear, though.

Isn't this just a habit born out of portability concerns?

View on HN

The alternative is to use a separate variable:

    y = realloc(x, ...);

Then if it fails, you can still free(x) before continuing. Of course, if you're going to exit() due to the failure it doesn't matter and perhaps that's why the original code looks that way. I still think it's worth pointing out, though.

About sizeof (char), there can be no "portability concerns", and that very thing is what I think gives rise to a lot of (in my opinion, bad) cargo cult programming. The value of "sizeof (char)" in C is 1. Note that I didn't say "using GCC x.y.z on x86". It's true for all conforming compilers, on all platforms, and on all architectures. It's defined by the language.

  > The value of "sizeof (char)" in C is 1.

For those who might not be aware, this does not mean that a char is always 8 bits.

According to C99 spec, the maximum number of bits for the smallest object that is not a bit-field is 8 bits. See "CHAR_BIT" in the C99 spec.

[1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf

Smallest. It might be larger (and infact it is in a lot of DSP toolchains)

Regarding realloc:

    y = realloc(x, ...)
    if (y == null) {
       free(x);
       // Handle OOM situation
    }
    x = y;

> What would the alternative be here? Allocate a new region, copy everything over and free the other region? Or is there something better?

No.

If realloc fails it returns NULL. In which case the original `x` is still valid. ==> However with `x = realloc(x,...)` you don't don't have a reference to `x` any more!

Reply on news.ycombinator.com