Re: safer node casting

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-01-02T08:40:23Z
Lists: pgsql-hackers
Hi,


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));


> I propose a macro castNode() that combines the assertion and the cast,
> so this would become
> 
>     RestrictInfo *rinfo = castNode(RestrictInfo, lfirst(lc));

I'm quite a bit in favor of something like this, having proposed it
before ;)

> +#define castNode(_type_,nodeptr)	(AssertMacro(!nodeptr || IsA(nodeptr,_type_)), (_type_ *)(nodeptr))

ISTM that we need to do the core part of this in an inline function, to
avoid multiple evaluation hazards - which seem quite likely to occur
here - it's pretty common to cast the result of a function after all.

Something like

static inline Node*
castNodeImpl(void *c, enum NodeTag t)
{
    Assert(c == NULL || IsA(c, t));
    return c;
}

#define castNode(_type_, nodeptr) ((_type_ *) castNodeImpl(nodeptr, _type_))

should work without too much trouble afaics?

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.