Re: GinPageIs* don't actually return a boolean - Mailing list pgsql-hackers

From Andres Freund
Subject Re: GinPageIs* don't actually return a boolean
Date
Msg-id 20160327155218.invhpni2xrmglikf@alap3.anarazel.de
Whole thread Raw
In response to Re: GinPageIs* don't actually return a boolean  (Yury Zhuravlev <u.zhuravlev@postgrespro.ru>)
List pgsql-hackers
On 2016-03-18 14:36:23 +0300, Yury Zhuravlev wrote:
> Robert Haas wrote:
> >On Wed, Mar 2, 2016 at 9:48 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
> >>On 2/11/16 9:30 PM, Michael Paquier wrote: ...
> >
> >We need to decide what to do about this.  I disagree with Peter: I
> >think that regardless of stdbool, what we've got right now is sloppy
> >coding - bad style if nothing else.  Furthermore, I think that while C
> >lets you use any non-zero value to represent true, our bool type is
> >supposed to contain only one of those two values.  Therefore, I think
> >we should commit the full patch, back-patch it as far as somebody has
> >the energy for, and move on.  But regardless, this patch can't keep
> >sitting in the CommitFest - we either have to take it or reject it,
> >and soon.
> >
> 
> I know that we are trying to do the right thing. But right now there is an
> error only in ginStepRight. Maybe now the fix this place, and we will think
> about "bool" then? The patch is attached (small and simple).
> 
> Thanks.
> 
> 
> -- 
> Yury Zhuravlev
> Postgres Professional: http://www.postgrespro.com
> The Russian Postgres Company

> diff --git a/src/backend/access/gin/ginbtree.c b/src/backend/access/gin/ginbtree.c
> index 06ba9cb..30113d0 100644
> --- a/src/backend/access/gin/ginbtree.c
> +++ b/src/backend/access/gin/ginbtree.c
> @@ -162,8 +162,8 @@ ginStepRight(Buffer buffer, Relation index, int lockmode)
>  {
>         Buffer          nextbuffer;
>         Page            page = BufferGetPage(buffer);
> -       bool            isLeaf = GinPageIsLeaf(page);
> -       bool            isData = GinPageIsData(page);
> +       uint8           isLeaf = GinPageIsLeaf(page);
> +       uint8           isData = GinPageIsData(page);
>         BlockNumber blkno = GinPageGetOpaque(page)->rightlink;
>  
>         nextbuffer = ReadBuffer(index, blkno);

I've pushed the gin specific stuff (although I fixed the macros instead
of the callsites) to all branches. I plan to commit the larger patch
(which has grown since last posting it) after the minor releases; it's
somewhat large and has a lot of conflicts. This way at least the
confirmed issue is fixed in the next set of minor releases.

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: AssertArg failure in src/backend/executor/functions.c:check_sql_fn_retval()
Next
From: Christophe Pettus
Date:
Subject: Re: Alter or rename enum value