Re: new heapcheck contrib module

> On Oct 22, 2020, at 1:00 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Thu, Oct 22, 2020 at 3:15 PM Mark Dilger
> <mark(dot)dilger(at)enterprisedb(dot)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't check 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 that was because the old arrangement seemed to make it harder to reason about when and where fxid got set. I think that is more clear now.

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