Horton

Re: safer node casting

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-02-24T12:31:27Z
Lists: pgsql-hackers 
Hi,

I was about to add a few more uses of castNode, which made me think.

You proposed replacing:

On 2016-12-31 12:08:22 -0500, Peter Eisentraut wrote:
> There is a common coding pattern that goes like this:
> 
>     RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
>     Assert(IsA(rinfo, RestrictInfo));

with

> +#define castNode(_type_,nodeptr)	(AssertMacro(!nodeptr || IsA(nodeptr,_type_)), (_type_ *)(nodeptr))
(now an inline function, but that's besides my point)

Those aren't actually equivalent, because of the !nodeptr. IsA() crashes
for NULL pointers, but the new code won't. Which means 9ba8a9ce4548b et
al actually weakened some asserts.

Should we perhaps have one NULL accepting version (castNodeNull?) and
one that separately asserts that ptr != NULL?

Greetings,

Andres Freund

Commits

  1. Suppress unused-variable warning.

  2. Make more use of castNode()

  3. Add castNode(type, ptr) for safe casting between NodeTag based types.

  4. Use the new castNode() macro in a number of places.