On Wed, Aug 18, 2021 at 6:30 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.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