Tidy up GetMultiXactIdMembers()'s behavior on error
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Thu, 17 Jun 2021 11:50:42 +0000 (14:50 +0300)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Thu, 17 Jun 2021 11:52:45 +0000 (14:52 +0300)
commit42f782be916807121fcce20811892c0bad95fca3
tree00dd3a3ef918cf3bdb186f690388ff03526d31d3
parent3989f8fb9dd84afda7e27de04bf83e31903d46ba
Tidy up GetMultiXactIdMembers()'s behavior on error

One of the error paths left *members uninitialized. That's not a live
bug, because most callers don't look at *members when the function
returns -1, but let's be tidy. One caller, in heap_lock_tuple(), does
"if (members != NULL) pfree(members)", but AFAICS it never passes an
invalid 'multi' value so it should not reach that error case.

The callers are also a bit inconsistent in their expectations.
heap_lock_tuple() pfrees the 'members' array if it's not-NULL, others
pfree() it if "nmembers >= 0", and others if "nmembers > 0". That's
not a live bug either, because the function should never return 0, but
add an Assert for that to make it more clear. I left the callers alone
for now.

I also moved the line where we set *nmembers. It wasn't wrong before,
but I like to do that right next to the 'return' statement, to make it
clear that it's always set on return.

Also remove one unreachable return statement after ereport(ERROR), for
brevity and for consistency with the similar if-block right after it.

Author: Greg Nancarrow with the additional changes by me
Backpatch-through: 9.6, all supported versions
src/backend/access/transam/multixact.c