Re: silence compiler warning in brin.c

On Wed, Jun 1, 2022 at 10:06 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Zhihong Yu <zyu@yugabyte.com> writes:
> > On Wed, Jun 1, 2022 at 9:35 AM Nathan Bossart <nathandbossart@gmail.com>
> > wrote:
> >> I'm seeing a compiler warning in brin.c with an older version of gcc.
> >> Specifically, it seems worried that a variable might not be initialized.
> >> AFAICT there is no real risk, so I've attached a small patch to silence
> the
> >> warning.
>
> Yeah, I noticed the other day that a couple of older buildfarm members
> (curculio, gaur) are grousing about this too.  We don't really have a
> hard-n-fast rule about how old a compiler needs to be before we stop
> worrying about its notions about uninitialized variables, but these are
> kind of old.  Still, since this is the only such warning from these
> animals, I'm inclined to silence it.
>
> > It seems the variable can be initialized to the value of GUCNestLevel
> since
> > later in the func:
> >     /* Roll back any GUC changes executed by index functions */
> >     AtEOXact_GUC(false, save_nestlevel);
>
> That seems pretty inappropriate.  If, thanks to some future thinko,
> control were able to reach the AtEOXact_GUC call despite not having
> called NewGUCNestLevel, we'd want that to fail.  It looks like
> AtEOXact_GUC asserts nestLevel > 0, so that either 0 or -1 would
> do as an "invalid" value; I'd lean a bit to using 0.
>
>                         regards, tom lane
>
Hi,

    if (heapRel == NULL || heapoid != IndexGetRelation(indexoid, false))
        ereport(ERROR,

I wonder why the above check is not placed in the else block:

    else
        heapRel = NULL;

because heapRel is not modified between the else and the above check.
If the check is placed in the else block, we can potentially save the call
to index_open().

Cheers