Fix child-address counting.
authorJeremy Harris <jgh146exb@wizmail.org>
Tue, 28 Feb 2017 18:24:40 +0000 (18:24 +0000)
committerJeremy Harris <jgh146exb@wizmail.org>
Tue, 28 Feb 2017 18:59:22 +0000 (18:59 +0000)
When a new address was created by a routing step it was possible for the parent address in the tree to
be marked as having zero children, despite the new child having a pointer to the parent.  When the child
was then delivered, the count on the parent could go negative or, if other children had been added which
correctly incremented the count, arrive at zero while some children were outstanding.  Fix this to
maintin the invariant.  While there, make the counter unsigned.

src/src/deliver.c
src/src/routers/iplookup.c
src/src/routers/queryprogram.c
src/src/routers/redirect.c
src/src/routers/rf_change_domain.c
src/src/structs.h

index c416ebf3fd78f79e11a864c8ae50eeb28fc82711..cb4616e6cd24143a3934e9c9792c02e41c86aefe 100644 (file)
@@ -671,7 +671,7 @@ address_item *aa;
 while (addr->parent)
   {
   addr = addr->parent;
-  if ((addr->child_count -= 1) > 0) return;   /* Incomplete parent */
+  if (--addr->child_count > 0) return;   /* Incomplete parent */
   address_done(addr, now);
 
   /* Log the completion of all descendents only when there is no ancestor with
@@ -2975,13 +2975,13 @@ while (addr_local)
        addr3 = store_get(sizeof(address_item));
        *addr3 = *addr2;
        addr3->next = NULL;
-       addr3->shadow_message = (uschar *) &(addr2->shadow_message);
+       addr3->shadow_message = US &addr2->shadow_message;
        addr3->transport = stp;
        addr3->transport_return = DEFER;
        addr3->return_filename = NULL;
        addr3->return_file = -1;
        *last = addr3;
-       last = &(addr3->next);
+       last = &addr3->next;
        }
 
     /* If we found any addresses to shadow, run the delivery, and stick any
@@ -5031,6 +5031,7 @@ if (percent_hack_domains)
     address_item *new_parent = store_get(sizeof(address_item));
     *new_parent = *addr;
     addr->parent = new_parent;
+    new_parent->child_count = 1;
     addr->address = new_address;
     addr->unique = string_copy(new_address);
     addr->domain = deliver_domain;
@@ -5901,9 +5902,9 @@ else if (system_filter && process_recipients != RECIP_FAIL_TIMEOUT)
 
     while (p)
       {
-      if (parent->child_count == SHRT_MAX)
+      if (parent->child_count == USHRT_MAX)
         log_write(0, LOG_MAIN|LOG_PANIC_DIE, "system filter generated more "
-          "than %d delivery addresses", SHRT_MAX);
+          "than %d delivery addresses", USHRT_MAX);
       parent->child_count++;
       p->parent = parent;
 
@@ -7141,10 +7142,9 @@ for (addr_dsntmp = addr_succeed; addr_dsntmp; addr_dsntmp = addr_dsntmp->next)
      )
     {
     /* copy and relink address_item and send report with all of them at once later */
-    address_item *addr_next;
-    addr_next = addr_senddsn;
+    address_item * addr_next = addr_senddsn;
     addr_senddsn = store_get(sizeof(address_item));
-    memcpy(addr_senddsn, addr_dsntmp, sizeof(address_item));
+    *addr_senddsn = *addr_dsntmp;
     addr_senddsn->next = addr_next;
     }
   else
index e6a35a7f3b4174fd4e71f5fd1d7f5821b993887e..96e9626df63236d37e80deddfcad1dfd7c826fd5 100644 (file)
@@ -382,9 +382,9 @@ new_addr->parent = addr;
 copyflag(new_addr, addr, af_propagate);
 new_addr->prop = addr->prop;
 
-if (addr->child_count == SHRT_MAX)
+if (addr->child_count == USHRT_MAX)
   log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s router generated more than %d "
-    "child addresses for <%s>", rblock->name, SHRT_MAX, addr->address);
+    "child addresses for <%s>", rblock->name, USHRT_MAX, addr->address);
 addr->child_count++;
 new_addr->next = *addr_new;
 *addr_new = new_addr;
index bfcaefcfd88111a371cf23c06c72e7dc028b50b1..cd02f366f1ab3d697094204fba521d1f9374c65c 100644 (file)
@@ -120,9 +120,9 @@ while (generated != NULL)
   next->next = *addr_new;
   *addr_new = next;
 
-  if (addr->child_count == SHRT_MAX)
+  if (addr->child_count == USHRT_MAX)
     log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s router generated more than %d "
-      "child addresses for <%s>", rblock->name, SHRT_MAX, addr->address);
+      "child addresses for <%s>", rblock->name, USHRT_MAX, addr->address);
   addr->child_count++;
 
   DEBUG(D_route)
index 8aad1d4abd4fa1ac391e3f0f788486f10c567076..29537baaee5bc46359a782f1fa4c913055be8e8e 100644 (file)
@@ -335,9 +335,9 @@ while (generated)
   next->parent = addr;
   orflag(next, addr, af_ignore_error);
   next->start_router = rblock->redirect_router;
-  if (addr->child_count == SHRT_MAX)
+  if (addr->child_count == USHRT_MAX)
     log_write(0, LOG_MAIN|LOG_PANIC_DIE, "%s router generated more than %d "
-      "child addresses for <%s>", rblock->name, SHRT_MAX, addr->address);
+      "child addresses for <%s>", rblock->name, USHRT_MAX, addr->address);
   addr->child_count++;
 
   next->next = *addr_new;
index 8986925c18f542a85c25f1d75839ccbd2730ead2..219e283ccbb55eaac83412f1c2c9f2db699b8213 100644 (file)
@@ -57,6 +57,7 @@ addr->prop = parent->prop;
 addr->address = address;
 addr->unique = string_copy(address);
 addr->parent = parent;
+parent->child_count = 1;
 
 addr->next = *addr_new;
 *addr_new = addr;
index d9d37f1c028c41871d43293d130b38658df82865..38b095f06a584e185f5d0ba1e072763f107dc8e2 100644 (file)
@@ -625,7 +625,7 @@ typedef struct address_item {
                                   /* (may need to hold a timestamp) */
 
   short int basic_errno;          /* status after failure */
-  short int child_count;          /* number of child addresses */
+  unsigned short child_count;     /* number of child addresses */
   short int return_file;          /* fileno of return data file */
   short int special_action;       /* ( used when when deferred or failed */
                                   /* (  also  */