From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
Cc: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: safer node casting |
Date: | 2017-01-04 08:10:44 |
Message-ID: | 20170104081044.zvqbl4dosqrdwvrg@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2017-01-03 11:00:47 +0530, Ashutosh Bapat wrote:
> On Mon, Jan 2, 2017 at 2:10 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > 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?
> >
> I tried this quickly as per attached patch. It gave a compiler error
> createplan.c: In function ‘castNodeImpl’:
> createplan.c:340:2: error: ‘T_t’ undeclared (first use in this function)
> createplan.c:340:2: note: each undeclared identifier is reported only
> once for each function it appears in
> createplan.c: In function ‘create_plan_recurse’:
> createplan.c:445:13: error: expected expression before ‘AggPath’
Well, I wrote that just to outline my suggestion, not as a patch ;).
It's just that we have to replace IsA() with nodeTag(nodeptr) == t
(because IsA does string concat magic).
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Craig Ringer | 2017-01-04 08:16:18 | Re: [PATCH] PostgresNode.pm enhancements, pg_lsn helper, and some more recovery tests |
Previous Message | Andres Freund | 2017-01-04 08:05:38 | Re: pgsql: Update copyright for 2017 |