Re: Possible dereference null return (src/backend/replication/logical/reorderbuffer.c)

Hi,
How about the following patch ?

ReorderBufferSetBaseSnapshot() can return a bool to indicate whether the
base snapshot is set up.

For the call by SnapBuildCommitTxn(), it seems xid is top transaction. So
the return value doesn't need to be checked.

Cheers

On Fri, Feb 12, 2021 at 6:40 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> On Fri, Feb 12, 2021 at 03:56:02PM +0900, Kyotaro Horiguchi wrote:
> > If the return from the first call is a subtransaction, the second call
> > always obtain the top transaction. If the top transaction actualy did
> > not exist, it's rather the correct behavior to cause SEGV, than
> > creating a bogus rbtxn. THus it is wrong to set create=true and
> > create_as_top=true. We could change the assertion like Assert (txn &&
> > txn->base_snapshot) to make things clearer.
>
> I don't see much the point to change this code. The result would be
> the same: a PANIC at this location.
> --
> Michael
>