pg_bsd_indent - improvements around offsetof and sizeof

From: Piotr Stefaniak <postgres(at)piotr-stefaniak(dot)me>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>
Subject: pg_bsd_indent - improvements around offsetof and sizeof
Date: 2016-05-22 20:16:39
Message-ID: BLU436-SMTP19ACD22D6308DB99B395F1F24D0@phx.gbl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello,

I think I've managed to improve pg_bsd_indent's handling of two types of
cases.

The first are like in this example:
- hashp = (HTAB *) DynaHashAlloc(sizeof(HTAB) + strlen(tabname) +1);
+ hashp = (HTAB *) DynaHashAlloc(sizeof(HTAB) + strlen(tabname) + 1);
Pristine pg_bsd_indent is inconsistent in masking parentheses as those
that are part of a cast and those that "are part of sizeof": seeing a
type name following an lparen it always masks that lparen as a part of a
cast; seeing an rparen it only removes the bit if it doesn't overlap
with sizeof_mask. In the example above, "(HTAB" started both "cast
parens" and "sizeof parens" at the same time, and the immediately
following rparen ended only the "sizeof parens". According to indent,
the cast-to type then ends at "tabname)" and what follows is the cast's
operand, including the + operator; in that case it's assumed to be unary
and not binary, which is why indent doesn't add the space after it.
The fix was to make it consistent about masking parens:
- ps.cast_mask |= 1 << ps.p_l_follow;
+ ps.cast_mask |= (1 << ps.p_l_follow & ~ps.sizeof_mask);

The second type of cases are like this:
- nse = palloc(offsetof(PLpgSQL_nsitem, name) +strlen(name) + 1);
+ nse = palloc(offsetof(PLpgSQL_nsitem, name) + strlen(name) + 1);
pg_bsd_indent simply hasn't been taught that a parenthesized type name
following the offsetof macro and then an lparen is another exception to
the rule of thumb that a construction like that generally means a cast.

You'll also notice other, seemingly unrelated changes, most notably the
rearrangement in numbers assigned to keywords. I've done it that way so
that it was easier and simpler to keep the -bs option functioning as
designed.

I've also renamed "sizeof_mask" to "not_cast_mask", because I think the
latter is a better description of what the mask does (it prevents
interpreting parenthesized type names as a cast where they aren't,
namely where they follow sizeof or offsetof; I haven't done any support
for function declarators and I don't plan to - the fact that
pg_bsd_indent thinks that "(int" in "char func(int);" begins a cast is
amusing but it seems harmless for now).

I'm attaching the patch for pg_bsd_indent and also a full diff that
shows the change in its behavior when run against PG's sources.

Attachment Content-Type Size
pg_bsd_indent-sizeof-offsetof-fix.diff text/x-patch 5.5 KB
effect_on_pg.diff text/x-patch 19.9 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2016-05-22 21:15:55 Re: Parallel query
Previous Message David G. Johnston 2016-05-22 19:56:49 Re: Adding an alternate syntax for Phrase Search