Transform string_append_listele{,_n}() to proper expanding-string triplet interface

(but do always maintain a nul-term string result).  This avoids always copying the
previous list version, and should do fewer allocs too.

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 8c6a1a5d3a82775a927fc8b31d0739004f556689..7544e35a4962c89d9a3d1160f1df026e1d5c82c5 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -115,6 +115,9 @@ PP/06 Repair manualroute's ability to take options in any order, even if one
 
 HS/01 Cleanup, prevent repeated use of -p/-oMr (CVE-2017-1000369)
 
+JH/17 Change the list-building routines interface to use the expanding-string
+      triplet model, for better allocation and copying behaviour.
+
 
 Exim version 4.89
 -----------------

diff --git a/src/src/daemon.c b/src/src/daemon.c
index 06c2b258c11c693e6b71055266788702536312f2..a05b79723bd8723d4971404d6493b0f5f5e41414 100644 (file)
--- a/src/src/daemon.c
+++ b/src/src/daemon.c
@@ -1173,6 +1173,8 @@ if (daemon_listen && !inetd_wait_mode)
   while ((s = string_nextinlist(&list, &sep, big_buffer, big_buffer_size)))
     if (!isdigit(*s))
       {
+      int size = 0, len = 0;
+
       list = tls_in.on_connect_ports;
       tls_in.on_connect_ports = NULL;
       sep = 0;
@@ -1180,13 +1182,13 @@ if (daemon_listen && !inetd_wait_mode)
        {
         if (!isdigit(*s))
          {
-         struct servent *smtp_service = getservbyname(CS s, "tcp");
+         struct servent * smtp_service = getservbyname(CS s, "tcp");
          if (!smtp_service)
            log_write(0, LOG_PANIC_DIE|LOG_CONFIG, "TCP port \"%s\" not found", s);
-         s= string_sprintf("%d", (int)ntohs(smtp_service->s_port));
+         s = string_sprintf("%d", (int)ntohs(smtp_service->s_port));
          }
        tls_in.on_connect_ports = string_append_listele(tls_in.on_connect_ports,
-           ':', s);
+           &size, &len, ':', s);
        }
       break;
       }

diff --git a/src/src/dkim.c b/src/src/dkim.c
index 03efb18d4c88de1b5dbd1c6f6fcbc3a93b8ea7c1..33e7bc84cdc134d89111f7d4ece0e8d9d4c41bb6 100644 (file)
--- a/src/src/dkim.c
+++ b/src/src/dkim.c
@@ -251,10 +251,12 @@ for (sig = dkim_signatures; sig; sig = sig->next)
   /* Build a colon-separated list of signing domains (and identities, if present) in dkim_signers */
 
   if (sig->domain)
-    dkim_signers = string_append_listele(dkim_signers, ':', sig->domain);
+    dkim_signers = string_append_listele(dkim_signers, &dkim_signers_size,
+      &dkim_signers_ptr, ':', sig->domain);
 
   if (sig->identity)
-    dkim_signers = string_append_listele(dkim_signers, ':', sig->identity);
+    dkim_signers = string_append_listele(dkim_signers, &dkim_signers_size,
+      &dkim_signers_ptr, ':', sig->identity);
 
   /* Process next signature */
   }

diff --git a/src/src/expand.c b/src/src/expand.c
index a68849dc34d7ab56ccb220933ea525cc19f212b4..a064e34e462db5c90ff4064a03fffa2b7f74f3ba 100644 (file)
--- a/src/src/expand.c
+++ b/src/src/expand.c
@@ -5985,7 +5985,9 @@ while (*s != 0)
         {
        uschar * dstitem;
        uschar * newlist = NULL;
+       int size = 0, len = 0;
        uschar * newkeylist = NULL;
+       int ksize = 0, klen = 0;
        uschar * srcfield;
 
         DEBUG(D_expand) debug_printf_indent("%s: $item = \"%s\"\n", name, srcitem);
@@ -6030,33 +6032,33 @@ while (*s != 0)
            /* New-item sorts before this dst-item.  Append new-item,
            then dst-item, then remainder of dst list. */
 
-           newlist = string_append_listele(newlist, sep, srcitem);
-           newkeylist = string_append_listele(newkeylist, sep, srcfield);
+           newlist = string_append_listele(newlist, &size, &len, sep, srcitem);
+           newkeylist = string_append_listele(newkeylist, &ksize, &klen, sep, srcfield);
            srcitem = NULL;
 
-           newlist = string_append_listele(newlist, sep, dstitem);
-           newkeylist = string_append_listele(newkeylist, sep, dstfield);
+           newlist = string_append_listele(newlist, &size, &len, sep, dstitem);
+           newkeylist = string_append_listele(newkeylist, &ksize, &klen, sep, dstfield);
 
            while ((dstitem = string_nextinlist(&dstlist, &sep, NULL, 0)))
              {
              if (!(dstfield = string_nextinlist(&dstkeylist, &sep, NULL, 0)))
                goto sort_mismatch;
-             newlist = string_append_listele(newlist, sep, dstitem);
-             newkeylist = string_append_listele(newkeylist, sep, dstfield);
+             newlist = string_append_listele(newlist, &size, &len, sep, dstitem);
+             newkeylist = string_append_listele(newkeylist, &ksize, &klen, sep, dstfield);
              }
 
            break;
            }
 
-         newlist = string_append_listele(newlist, sep, dstitem);
-         newkeylist = string_append_listele(newkeylist, sep, dstfield);
+         newlist = string_append_listele(newlist, &size, &len, sep, dstitem);
+         newkeylist = string_append_listele(newkeylist, &ksize, &klen, sep, dstfield);
          }
 
        /* If we ran out of dstlist without consuming srcitem, append it */
        if (srcitem)
          {
-         newlist = string_append_listele(newlist, sep, srcitem);
-         newkeylist = string_append_listele(newkeylist, sep, srcfield);
+         newlist = string_append_listele(newlist, &size, &len, sep, srcitem);
+         newkeylist = string_append_listele(newkeylist, &ksize, &klen, sep, srcfield);
          }
 
        dstlist = newlist;

diff --git a/src/src/functions.h b/src/src/functions.h
index 34f6434d0c1a28fc28c544dc5a4ebf498c8fc5e3..41e3ec163d99a1625b99ae255a8469949ef72bd1 100644 (file)
--- a/src/src/functions.h
+++ b/src/src/functions.h
@@ -440,8 +440,8 @@ extern int     stdin_feof(void);
 extern int     stdin_ferror(void);
 extern int     stdin_ungetc(int);
 extern uschar *string_append(uschar *, int *, int *, int, ...) WARN_UNUSED_RESULT;
-extern uschar *string_append_listele(uschar *, uschar, const uschar *);
-extern uschar *string_append_listele_n(uschar *, uschar, const uschar *, unsigned);
+extern uschar *string_append_listele(uschar *, int *, int *, uschar, const uschar *) WARN_UNUSED_RESULT;
+extern uschar *string_append_listele_n(uschar *, int *, int *, uschar, const uschar *, unsigned) WARN_UNUSED_RESULT;
 extern uschar *string_base62(unsigned long int);
 extern uschar *string_cat(uschar *, int *, int *, const uschar *) WARN_UNUSED_RESULT;
 extern uschar *string_catn(uschar *, int *, int *, const uschar *, int) WARN_UNUSED_RESULT;

diff --git a/src/src/readconf.c b/src/src/readconf.c
index fd9657e0e763d0d3035fd22aa2bee32730fa57a5..eb44d1085b2ac7e4881804f8908c2141eb5ba275 100644 (file)
--- a/src/src/readconf.c
+++ b/src/src/readconf.c
@@ -1899,9 +1899,13 @@ switch (type)
       const uschar * list = sptr;
       uschar * s;
       uschar * list_o = *str_target;
+      int size = 0, len = 0;
+
+      if (list_o)
+       size = (len = Ustrlen(list_o)) + 1;
 
       while ((s = string_nextinlist(&list, &sep_i, NULL, 0)))
-       list_o = string_append_listele(list_o, sep_o, s);
+       list_o = string_append_listele(list_o, &size, &len, sep_o, s);
       if (list_o)
        *str_target = string_copy_malloc(list_o);
       }

diff --git a/src/src/routers/rf_get_munge_headers.c b/src/src/routers/rf_get_munge_headers.c
index ecb4ee097de69680e2ec4b89e6d878305716d1f8..745704f6241ddb112a4af50957dca8f5cc8d481c 100644 (file)
--- a/src/src/routers/rf_get_munge_headers.c
+++ b/src/src/routers/rf_get_munge_headers.c
@@ -91,6 +91,10 @@ if (rblock->remove_headers)
   const uschar * list = rblock->remove_headers;
   int sep = ':';
   uschar * s;
+  int size = 0, len = 0;
+
+  if (*remove_headers)
+    size = (len = Ustrlen(*remove_headers)) + 1;
 
   while ((s = string_nextinlist(&list, &sep, NULL, 0)))
     if (!(s = expand_string(s)))
@@ -104,7 +108,7 @@ if (rblock->remove_headers)
        }
       }
     else if (*s)
-      *remove_headers = string_append_listele(*remove_headers, ':', s);
+      *remove_headers = string_append_listele(*remove_headers, &size, &len, ':', s);
   }
 
 return OK;

diff --git a/src/src/string.c b/src/src/string.c
index 0d5a09703452a764d4dade82d82108d39bc70659..53bcdfb7b7795ab20101892a6191418eee63977f 100644 (file)
--- a/src/src/string.c
+++ b/src/src/string.c
@@ -968,21 +968,43 @@ else
 *listptr = s;
 return buffer;
 }
-#endif  /* COMPILE_UTILITY */
 
 
-#ifndef COMPILE_UTILITY
+static const uschar *
+Ustrnchr(const uschar * s, int c, unsigned * len)
+{
+unsigned siz = *len;
+while (siz)
+  {
+  if (!*s) return NULL;
+  if (*s == c)
+    {
+    *len = siz;
+    return s;
+    }
+  s++;
+  siz--;
+  }
+return NULL;
+}
+
+
 /************************************************
 *      Add element to separated list           *
 ************************************************/
-/* This function is used to build a list, returning
-an allocated null-terminated growable string. The
-given element has any embedded separator characters
+/* This function is used to build a list, returning an allocated null-terminated
+growable string. The given element has any embedded separator characters
 doubled.
 
+Despite having the same growable-string interface as string_cat() the list is
+always returned null-terminated.
+
 Arguments:
   list points to the start of the list that is being built, or NULL
        if this is a new list that has no contents yet
+  sz    (ptr to) amount of memory allocated for list; zero for a new list
+  off   (ptr to) current list length in chars (insert point for next addition),
+        zero for a new list
   sep  list separator character
   ele  new element to be appended to the list
 
@@ -990,78 +1012,49 @@ Returns:  pointer to the start of the list, changed if copied for expansion.
 */
 
 uschar *
-string_append_listele(uschar * list, uschar sep, const uschar * ele)
+string_append_listele(uschar * list, int * sz, int * off,
+  uschar sep, const uschar * ele)
 {
-uschar * new = NULL;
-int sz = 0, off = 0;
 uschar * sp;
 
 if (list)
-  {
-  new = string_cat (new, &sz, &off, list);
-  new = string_catn(new, &sz, &off, &sep, 1);
-  }
+  list = string_catn(list, sz, off, &sep, 1);
 
 while((sp = Ustrchr(ele, sep)))
   {
-  new = string_catn(new, &sz, &off, ele, sp-ele+1);
-  new = string_catn(new, &sz, &off, &sep, 1);
+  list = string_catn(list, sz, off, ele, sp-ele+1);
+  list = string_catn(list, sz, off, &sep, 1);
   ele = sp+1;
   }
-new = string_cat(new, &sz, &off, ele);
-new[off] = '\0';
-return new;
+list = string_cat(list, sz, off, ele);
+list[*off] = '\0';
+return list;
 }
 
 
-static const uschar *
-Ustrnchr(const uschar * s, int c, unsigned * len)
-{
-unsigned siz = *len;
-while (siz)
-  {
-  if (!*s) return NULL;
-  if (*s == c)
-    {
-    *len = siz;
-    return s;
-    }
-  s++;
-  siz--;
-  }
-return NULL;
-}
-
 uschar *
-string_append_listele_n(uschar * list, uschar sep, const uschar * ele,
-  unsigned len)
+string_append_listele_n(uschar * list, int * sz, int * off,
+  uschar sep, const uschar * ele, unsigned len)
 {
-uschar * new = NULL;
-int sz = 0, off = 0;
 const uschar * sp;
 
 if (list)
-  {
-  new = string_cat (new, &sz, &off, list);
-  new = string_catn(new, &sz, &off, &sep, 1);
-  }
+  list = string_catn(list, sz, off, &sep, 1);
 
 while((sp = Ustrnchr(ele, sep, &len)))
   {
-  new = string_catn(new, &sz, &off, ele, sp-ele+1);
-  new = string_catn(new, &sz, &off, &sep, 1);
+  list = string_catn(list, sz, off, ele, sp-ele+1);
+  list = string_catn(list, sz, off, &sep, 1);
   ele = sp+1;
   len--;
   }
-new = string_catn(new, &sz, &off, ele, len);
-new[off] = '\0';
-return new;
+list = string_catn(list, sz, off, ele, len);
+list[*off] = '\0';
+return list;
 }
-#endif  /* COMPILE_UTILITY */
 
 
 
-#ifndef COMPILE_UTILITY
 /*************************************************
 *             Add chars to string                *
 *************************************************/

diff --git a/src/src/tls.c b/src/src/tls.c
index a5cb35bd9bba039f03478edf4fb1b2634dc192a5..c93eb4579069e36365aba99a222aa3e12bf038b0 100644 (file)
--- a/src/src/tls.c
+++ b/src/src/tls.c
@@ -264,6 +264,7 @@ uschar * ele;
 uschar * match = NULL;
 int len;
 uschar * list = NULL;
+int size = 0, pos = 0;
 
 while ((ele = string_nextinlist(&mod, &insep, NULL, 0)))
   if (ele[0] != '>')
@@ -278,7 +279,7 @@ while ((ele = string_nextinlist(CUSS &dn, &insep, NULL, 0)))
   if (  !match
      || Ustrncmp(ele, match, len) == 0 && ele[len] == '='
      )
-    list = string_append_listele(list, outsep, ele+len+1);
+    list = string_append_listele(list, &size, &pos, outsep, ele+len+1);
 return list;
 }
 

diff --git a/src/src/tlscert-gnu.c b/src/src/tlscert-gnu.c
index 296398ae9af2c878bdd6cf8badd322bbbab0c896..3c6953a9900ee65022e8305a257c01088fe1d7b8 100644 (file)
--- a/src/src/tlscert-gnu.c
+++ b/src/src/tlscert-gnu.c
@@ -280,6 +280,7 @@ uschar *
 tls_cert_subject_altname(void * cert, uschar * mod)
 {
 uschar * list = NULL;
+int lsize = 0, llen = 0;
 int index;
 size_t siz;
 int ret;
@@ -332,7 +333,7 @@ for(index = 0;; index++)
     case GNUTLS_SAN_RFC822NAME: tag = US"MAIL"; break;
     default: continue;        /* ignore unrecognised types */
     }
-  list = string_append_listele(list, sep,
+  list = string_append_listele(list, &lsize, &llen, sep,
           match == -1 ? string_sprintf("%s=%s", tag, ele) : ele);
   }
 /*NOTREACHED*/
@@ -347,6 +348,7 @@ int ret;
 uschar sep = '\n';
 int index;
 uschar * list = NULL;
+int lsize = 0, llen = 0;
 
 if (mod)
   if (*mod == '>' && *++mod) sep = *mod++;
@@ -361,8 +363,8 @@ for(index = 0;; index++)
   if (ret < 0)
     return g_err("gai", __FUNCTION__, ret);
 
-  list = string_append_listele(list, sep,
-           string_copyn(uri.data, uri.size));
+  list = string_append_listele_n(list, &lsize, &llen, sep,
+           uri.data, uri.size);
   }
 /*NOTREACHED*/
 
@@ -384,6 +386,7 @@ size_t siz;
 uschar sep = '\n';
 int index;
 uschar * list = NULL;
+int lsize = 0, llen = 0;
 uschar * ele;
 
 if (mod)
@@ -403,13 +406,12 @@ for(index = 0;; index++)
       return g_err("gc0", __FUNCTION__, ret);
     }
 
-  ele = store_get(siz+1);
+  ele = store_get(siz);
   if ((ret = gnutls_x509_crt_get_crl_dist_points(
       (gnutls_x509_crt_t)cert, index, ele, &siz, NULL, NULL)) < 0)
     return g_err("gc1", __FUNCTION__, ret);
 
-  ele[siz] = '\0';
-  list = string_append_listele(list, sep, ele);
+  list = string_append_listele_n(list, &lsize, &llen, sep, ele, siz);
   }
 /*NOTREACHED*/
 }

diff --git a/src/src/tlscert-openssl.c b/src/src/tlscert-openssl.c
index 690f9508148609b4572c587c02388c9ca4509215..2e6ee8b3ccd0ee3e1d5eab047068abbdfc24db67 100644 (file)
--- a/src/src/tlscert-openssl.c
+++ b/src/src/tlscert-openssl.c
@@ -344,6 +344,7 @@ uschar *
 tls_cert_subject_altname(void * cert, uschar * mod)
 {
 uschar * list = NULL;
+int lsize = 0, llen = 0;
 STACK_OF(GENERAL_NAME) * san = (STACK_OF(GENERAL_NAME) *)
   X509_get_ext_d2i((X509 *)cert, NID_subject_alt_name, NULL, NULL);
 uschar osep = '\n';
@@ -394,7 +395,7 @@ while (sk_GENERAL_NAME_num(san) > 0)
     ele = string_copyn(ele, len);
 
   if (Ustrlen(ele) == len)     /* ignore any with embedded nul */
-    list = string_append_listele(list, osep,
+    list = string_append_listele(list, &lsize, &llen, osep,
          match == -1 ? string_sprintf("%s=%s", tag, ele) : ele);
   }
 
@@ -411,6 +412,7 @@ int adsnum = sk_ACCESS_DESCRIPTION_num(ads);
 int i;
 uschar sep = '\n';
 uschar * list = NULL;
+int size = 0, len = 0;
 
 if (mod)
   if (*mod == '>' && *++mod) sep = *mod++;
@@ -420,11 +422,9 @@ for (i = 0; i < adsnum; i++)
   ACCESS_DESCRIPTION * ad = sk_ACCESS_DESCRIPTION_value(ads, i);
 
   if (ad && OBJ_obj2nid(ad->method) == NID_ad_OCSP)
-    {
-    uschar * ele = ASN1_STRING_data(ad->location->d.ia5);
-    int len =  ASN1_STRING_length(ad->location->d.ia5);
-    list = string_append_listele_n(list, sep, ele, len);
-    }
+    list = string_append_listele_n(list, &size, &len, sep,
+      ASN1_STRING_data(ad->location->d.ia5),
+      ASN1_STRING_length(ad->location->d.ia5));
   }
 sk_ACCESS_DESCRIPTION_free(ads);
 return list;
@@ -441,6 +441,7 @@ int dpsnum = sk_DIST_POINT_num(dps);
 int i;
 uschar sep = '\n';
 uschar * list = NULL;
+int size = 0, len = 0;
 
 if (mod)
   if (*mod == '>' && *++mod) sep = *mod++;
@@ -457,11 +458,9 @@ if (dps) for (i = 0; i < dpsnum; i++)
       if (  (np = sk_GENERAL_NAME_value(names, j))
         && np->type == GEN_URI
         )
-       {
-       uschar * ele = ASN1_STRING_data(np->d.uniformResourceIdentifier);
-       int len =  ASN1_STRING_length(np->d.uniformResourceIdentifier);
-       list = string_append_listele_n(list, sep, ele, len);
-       }
+       list = string_append_listele_n(list, &size, &len,  sep,
+         ASN1_STRING_data(np->d.uniformResourceIdentifier),
+         ASN1_STRING_length(np->d.uniformResourceIdentifier));
     }
 sk_DIST_POINT_free(dps);
 return list;

diff --git a/src/src/utf8.c b/src/src/utf8.c
index 7b7b88f66ef462c4bdb7c1b3fed990cec8fdfd21..255383051ba010d5939bf3d5dc678af4e5bcbb70 100644 (file)
--- a/src/src/utf8.c
+++ b/src/src/utf8.c
@@ -98,6 +98,7 @@ string_domain_alabel_to_utf8(const uschar * alabel, uschar ** err)
 const uschar * label;
 int sep = '.';
 uschar * s = NULL;
+int size = 0, len = 0;
 
 while (label = string_nextinlist(&alabel, &sep, NULL, 0))
   if (  string_is_alabel(label)
@@ -105,7 +106,7 @@ while (label = string_nextinlist(&alabel, &sep, NULL, 0))
      )
     return NULL;
   else
-    s = string_append_listele(s, '.', label);
+    s = string_append_listele(s, &size, &len, '.', label);
 return s;
 
 #else