Re: new heapcheck contrib module

> On Oct 22, 2020, at 1:00 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Thu, Oct 22, 2020 at 3:15 PM Mark Dilger
> <mark.dilger@enterprisedb.com> wrote:
>> The 0001 attached patch addresses the -Werror=maybe-uninitialized problem.
>
> I am skeptical. Why so much code churn to fix a compiler warning? And
> even in the revised code, *status isn't set in all cases, so I don't
> see why this would satisfy the compiler. Even if it satisfies this
> particular compiler for some other reason, some other compiler is
> bound to be unhappy sometime. It's better to just arrange to set
> *status always, and use a dummy value in cases where it doesn't
> matter. Also, "return XID_BOUNDS_OK;;" has exceeded its recommended
> allowance of semicolons.

I think the compiler warning was about fxid not being set.  The callers pass NULL for status if they don't want status
checked,so writing *status unconditionally would be an error.  Also, if the xid being checked is out of bounds, we
can'tcheck the status of the xid in clog. 

As for the code churn, I probably refactored it a bit more than I needed to fix the compiler warning about fxid, but
thatwas because the old arrangement seemed to make it harder to reason about when and where fxid got set.  I think that
ismore clear now. 

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company