Re: arrayfuncs: fix read of uninitialized mem - Mailing list pgsql-patches

From Tom Lane
Subject Re: arrayfuncs: fix read of uninitialized mem
Date
Msg-id 13384.1095301172@sss.pgh.pa.us
Whole thread Raw
In response to Re: arrayfuncs: fix read of uninitialized mem  (Neil Conway <neilc@samurai.com>)
Responses Re: arrayfuncs: fix read of uninitialized mem  (Neil Conway <neilc@samurai.com>)
List pgsql-patches
Neil Conway <neilc@samurai.com> writes:
> Personally I like the style I used, because it makes one class of
> mistake more difficult -- getting the sizeof() wrong. Suppose the type
> of the variable you're allocating changes -- if you use

> ptr = malloc(sizeof(*ptr));

> then the code is still right, but with

> ptr = malloc(sizeof(some_type));

> you need to remember to update the sizeof(); worse, the compiler won't
> warn you about this.

IMHO both of the above are poor style, precisely because the compiler
can't warn you about it if the sizeof calculation doesn't match the
pointer variable's type.  I prefer

    ptr = (foo *) malloc(sizeof(foo));
or
    ptr = (foo *) malloc(n * sizeof(foo));
since here you will get a warning if ptr is typed as something other
than foo *.  Now admittedly you are still relying on two bits of code to
match each other, namely the two occurrences of "foo" in this command
--- but they are at least adjacent in the source code whereas the
declaration of ptr might be some distance away.

No doubt you'll say that "ptr" is duplicated close together in your
preferred version, but I don't think it scales nicely to
slightly-more-complex cases.  Consider for instance

    ptrs[i++] = malloc(sizeof(*ptrs[i++]));

This is going to confuse people no matter what.  Either you don't write
the exact same thing on both sides, or you are relying on the reader to
realize the funny semantics of sizeof() and know that i isn't really
bumped twice by the above (not to mention relying on the compiler to
implement that correctly...).

I guess at bottom it's the funny semantics of sizeof-applied-to-an-
lvalue-expression that I don't like.  I think sizeof(type decl) is much
more obviously a compile-time constant.  Possibly this is just a
hangover from programming in other languages that had the one but not
the other, but it's what I find understandable.

            regards, tom lane

pgsql-patches by date:

Previous
From: Neil Conway
Date:
Subject: Re: arrayfuncs: fix read of uninitialized mem
Next
From: Neil Conway
Date:
Subject: Re: arrayfuncs: fix read of uninitialized mem