On Tue, Jul 19, 2022 at 2:01 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Tue, Jul 19, 2022 at 4:35 PM Kyotaro Horiguchi > <horikyota....@gmail.com> wrote: > > > > > > > + Assert((xcnt > 0) && (xcnt == rb->catchange_ntxns)); > > > > (xcnt > 0) is obvious here (otherwise means dlist_foreach is broken..). > > (xcnt == rb->catchange_ntxns) is not what should be checked here. The > > assert just requires that catchange_txns and catchange_ntxns are > > consistent so it should be checked just after dlist_empty.. I think. > > > > If we want to check if catchange_txns and catchange_ntxns are > consistent, should we check (xcnt == rb->catchange_ntxns) as well, no? > This function requires the caller to use rb->catchange_ntxns as the > length of the returned array. I think this assertion ensures that the > actual length of the array is consistent with the length we > pre-calculated. >
Right, so, I think it is better to keep this assertion but remove (xcnt > 0) part as pointed out by Horiguchi-San. -- With Regards, Amit Kapila.
