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

On 2021-08-18 1:29 a.m., Kyotaro Horiguchi wrote:
> At Tue, 17 Aug 2021 17:04:44 +0900, Michael Paquier <michael@paquier.xyz> wrote in
>> On Fri, Jul 02, 2021 at 06:22:56PM -0300, Ranier Vilela wrote:
>>> Em qui., 1 de jul. de 2021 às 17:20, Mahendra Singh Thalor <
>>> mahi6run@gmail.com> escreveu:
>>>> Please can we try to hit this rare condition by any test case. If you have
>>>> any test cases, please share.
>> Yeah, this needs to be proved.  Are you sure that this change is
>> actually right?  The bottom of FreePageManagerPutInternal() has
>> assumptions that a page may not be found during a btree search, with
>> an index value used.
> 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.
After added the initialization of split_pages in patch 
fix_unitialized_var_index_freepage-v1.patch,

+        result->split_pages = 0;

it actually changed the assertion condition after the second time 
function call of FreePageBtreeSearch.
             FreePageBtreeSearch(fpm, first_page, &result);

             /*
              * The act of allocating pages for use in constructing our 
btree
              * should never cause any page to become more full, so the new
              * split depth should be no greater than the old one, and 
perhaps
              * less if we fortuitously allocated a chunk that freed up 
a slot
              * on the page we need to update.
              */
             Assert(result.split_pages <= fpm->btree_recycle_count);

Should we consider adding some test cases to make sure this assertion 
will still function properly?

>
> regards.
>
-- 
David

Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca