Re: new heapcheck contrib module - Mailing list pgsql-hackers

From Tom Lane
Subject Re: new heapcheck contrib module
Date
Msg-id 41306.1603398216@sss.pgh.pa.us
Whole thread Raw
In response to Re: new heapcheck contrib module  (Mark Dilger <mark.dilger@enterprisedb.com>)
Responses Re: new heapcheck contrib module  (Mark Dilger <mark.dilger@enterprisedb.com>)
List pgsql-hackers
... btw, having now looked more closely at get_xid_status(), I wonder
how come there aren't more compilers bitching about it, because it
is very very obviously broken.  In particular, the case of
requesting status for an xid that is BootstrapTransactionId or
FrozenTransactionId *will* fall through to perform
FullTransactionIdPrecedesOrEquals with an uninitialized fxid.

The fact that most compilers seem to fail to notice that is quite scary.
I suppose it has something to do with FullTransactionId being a struct,
which makes me wonder if that choice was quite as wise as we thought.

Meanwhile, so far as this code goes, I wonder why you don't just change it
to always set that value, ie

    XidBoundsViolation result;
    FullTransactionId fxid;
    FullTransactionId clog_horizon;

+    fxid = FullTransactionIdFromXidAndCtx(xid, ctx);
+
    /* Quick check for special xids */
    if (!TransactionIdIsValid(xid))
        result = XID_INVALID;
    else if (xid == BootstrapTransactionId || xid == FrozenTransactionId)
        result = XID_BOUNDS_OK;
    else
    {
        /* Check if the xid is within bounds */
-        fxid = FullTransactionIdFromXidAndCtx(xid, ctx);
        if (!fxid_in_cached_range(fxid, ctx))
        {


            regards, tom lane



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: new heapcheck contrib module
Next
From: Mark Dilger
Date:
Subject: Re: new heapcheck contrib module