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;
 }
 
 /* ----------------------------------------------------------------

Attachment: xact.c.patch
Description: xact.c.patch

Reply via email to