Re: arrayfuncs: fix read of uninitialized mem

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: Joe Conway <mail(at)joeconway(dot)com>, pgsql-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: arrayfuncs: fix read of uninitialized mem
Date: 2004-09-16 02:19:32
Message-ID: 13384.1095301172@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Neil Conway <neilc(at)samurai(dot)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

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Neil Conway 2004-09-16 03:00:03 Re: arrayfuncs: fix read of uninitialized mem
Previous Message Neil Conway 2004-09-15 23:18:17 Re: arrayfuncs: fix read of uninitialized mem