Hi,
Surely that "s->nChildXids > 0", protects s->childXids to be NULL!
But, when we exchange the test (s->nChildXids > 0) by (s->childXids != NULL), I believe we have the same protection,
because,if "s->childXids" is not NULL, "s->nChildXids" is > 0, naturally.
That way we can improve the function and avoid calling and setting unnecessarily!
Bonus: silent compiler warning potential null pointer derenferencing.
Best regards,
Ranier Vilela
--- \dll\postgresql-12.0\a\backend\access\transam\xact.c Mon Sep 30 17:06:55 2019
+++ xact.c Wed Nov 13 13:03:28 2019
@@ -1580,20 +1580,20 @@
*/
s->parent->childXids[s->parent->nChildXids] = XidFromFullTransactionId(s->fullTransactionId);
- if (s->nChildXids > 0)
+ if (s->childXids != NULL) {
memcpy(&s->parent->childXids[s->parent->nChildXids + 1],
s->childXids,
s->nChildXids * sizeof(TransactionId));
+ /* Release child's array to avoid leakage */
+ pfree(s->childXids);
+ /* We must reset these to avoid double-free if fail later in commit */
+ s->childXids = NULL;
+ s->nChildXids = 0;
+ s->maxChildXids = 0;
+ }
+ Assert(s->nChildXids == 0 && s->maxChildXids == 0);
s->parent->nChildXids = new_nChildXids;
-
- /* Release child's array to avoid leakage */
- if (s->childXids != NULL)
- pfree(s->childXids);
- /* We must reset these to avoid double-free if fail later in commit */
- s->childXids = NULL;
- s->nChildXids = 0;
- s->maxChildXids = 0;
}
/* ----------------------------------------------------------------
