Re: Fix uninitialized variable access (src/backend/utils/mmgr/freepage.c)

From: Greg Nancarrow <gregn4422(at)gmail(dot)com>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, Mahendra Singh Thalor <mahi6run(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix uninitialized variable access (src/backend/utils/mmgr/freepage.c)
Date: 2021-08-18 10:53:31
Message-ID: CAJcOf-efnjA2Q-Fwr5wWUStzYX6c229-ifD6J_hFO=XV=KE=KQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Aug 18, 2021 at 6:30 PM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> By a quick look, FreePageBtreeSearch is called only from
> FreePageManagerPutInternal at three points. The first one assumes that
> result.found == true, at the rest points are passed only when
> fpm->btree_depth > 0, i.e, fpm->btree_root is non-NULL.
>
> In short FreePageBtreeSeach is never called when fpm->btree_root is
> NULL. I don't think we need to fill-in other members since the
> contract of the function looks fine.
>
> It might be simpler to turn 'if (btp == NULL)' to an assertion.
>

Even if there are no current calls to FreePageBtreeSeach() where it
results in "btp == NULL", FreePageBtreeSeach() is obviously handling
the possibility of that condition, and I think it's poor form to
return with two uninitialized members in the result for that,
especially when the current code for the "!result.found" case can
reference those members, and the usual return point of
FreePageBtreeSeach() has all result members set, including
result.found==true and result.found=false cases.
At best it's inconsistent and confusing and it looks like a bug
waiting to happen, so I'm still in favor of the patch.

Regards,
Greg Nancarrow
Fujitsu Australia

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Dipesh Pandit 2021-08-18 11:05:23 Re: .ready and .done files considered harmful
Previous Message Tomas Vondra 2021-08-18 10:43:05 Re: Use extended statistics to estimate (Var op Var) clauses