Re: Use C99 designated initializers for some structs

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: David Steele <david(at)pgmasters(dot)net>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Use C99 designated initializers for some structs
Date: 2018-08-29 22:51:24
Message-ID: 27557.1535583084@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2018-08-29 15:51:07 -0500, David Steele wrote:
>> One thing: I'm not sure that excluding the InvalidOid assignment in the
>> TopTransactionStateData initializer is a good idea. That is, it's not clear
>> that InvalidOid is 0.
>> NULL, false, and 0 seem like no-brainers, but maybe it would be better to
>> explicitly include constants that we define that are not obviously 0, or
>> maybe just all of them.

> We rely on that in many other places imo. So I don't think it's worth
> starting to care about it here.

I agree that assuming that they're physically zeroes is OK from a
portability standpoint, because we'd have a whole lot of other issues
if they weren't. But I have a different point to make, which is that
it's fairly standard practice for us to initialize all fields of a struct
explicitly, even when setting them to zero/false/NULL. I don't think we
should walk away from that practice just because C99 offers a shiny new
syntax for doing so.

The main practical advantage I see to writing such "redundant" explicit
field initializations is that it aids greppability: when you're adding a
new field Y beside field X, grepping for X is a good way of finding the
places where you need to initialize/copy/write/read/generically-frob Y
too. Omitting mention of X just because you're implicitly initializing
it puts a big hole in the reliability of that technique.

As against that, of course, explicitly zeroing fields that you know very
well are already zero eats some cycles. I've occasionally wondered if
it's worth doing things like

mynode = makeNode(MyNodeType);
mynode->w = 42;
/* mynode->x = 0; */
/* mynode->y = NULL; */
mynode->z = ptr;

but that seems mighty ugly.

An argument I *don't* buy is that leaving out redundant field assignments
is a good savings of programmer time. It's not a useful savings to the
original developer, and it's a net negative for anyone who has to review
or modify such code later. I think it was Brooks who said something to
the effect that any programmer would happily type out every line of code
thrice over, if only that would guarantee no bugs. It doesn't, of course,
but there are virtues in verbosity nonetheless.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-08-30 00:13:52 Re: Postmaster doesn't send SIGTERM to bgworker during fast shutdown when pmState == PM_STARTUP
Previous Message David Rowley 2018-08-29 22:27:09 Re: speeding up planning with partitions