On Fri, Mar 10, 2023 at 4:36 AM Peter Smith <smithpb2...@gmail.com> wrote: > > During a recent code review, I was confused multiple times by the > toptxn member of ReorderBufferTXN, which is defined only for > sub-transactions. > > e.g. txn->toptxn member == NULL means the txn is a top level txn. > e.g. txn->toptxn member != NULL means the txn is not a top level txn > > It makes sense if you squint and read it slowly enough, but IMO it's > too easy to accidentally misinterpret the meaning when reading code > that uses this member. > > ~ > > Such code can be made easier to read just by introducing some simple macros: > > #define isa_toptxn(rbtxn) (rbtxn->toptxn == NULL) > #define isa_subtxn(rbtxn) (rbtxn->toptxn != NULL) > #define get_toptxn(rbtxn) (isa_subtxn(rbtxn) ? rbtxn->toptxn : rbtxn) > > ~ > > PSA a small patch that does this. >
I also find it will make code easier to read. So, +1 to the idea. I'll do the detailed review and test next week unless there are objections to the idea. -- With Regards, Amit Kapila.