Hi, I fould a potential gotcha when playing with clang's code analysis tool.

The concat_strings function silently stopped counting string lengths when
given more than 5 arguments. clang warned about potential garbage values in
the saved_lengths array, so I redid it with this approach.

All tests working ok with this patch.

This is my first patch to this list, by the way. I'd be happy to help out
more in the future.

Best regards,

/Pär Karlsson, Sweden

----

commit 2d855670e0e1fbe578506b376cdd40b0e465d3ef
Author: Pär Karlsson <feino...@gmail.com>
Date:   Thu Oct 16 21:41:36 2014 +0200

    Updated ChangeLog

diff --git a/src/ChangeLog b/src/ChangeLog
index 1c4e2d5..1e39475 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2014-10-16  Pär Karlsson  <feino...@gmail.com>
+
+       * utils.c (concat_strings): fixed arbitrary limit of 5 arguments to
+       function
+
 2014-05-03  Tim Ruehsen  <tim.rueh...@gmx.de>

        * retr.c (retrieve_url): fixed memory leak

commit 1fa9ff274dcb6e5a2dbbbc7d3fe2f139059c47f1
Author: Pär Karlsson <feino...@gmail.com>
Date:   Wed Oct 15 00:00:31 2014 +0200

    Generalized concat_strings argument length

      The concat_strings function seemed arbitrary to only accept a maximum
      of 5 arguments (the others were silently ignored).

      Also it had a potential garbage read of the values in the array.
      Updated with xmalloc/xrealloc/free

diff --git a/src/utils.c b/src/utils.c
index 78c282e..93c9ddc 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -356,7 +356,8 @@ char *
 concat_strings (const char *str0, ...)
 {
   va_list args;
-  int saved_lengths[5];         /* inspired by Apache's apr_pstrcat */
+  size_t psize = sizeof(int);
+  int *saved_lengths = xmalloc (psize);
   char *ret, *p;

   const char *next_str;
@@ -370,8 +371,8 @@ concat_strings (const char *str0, ...)
   for (next_str = str0; next_str != NULL; next_str = va_arg (args, char *))
     {
       int len = strlen (next_str);
-      if (argcount < countof (saved_lengths))
-        saved_lengths[argcount++] = len;
+      saved_lengths[argcount++] = len;
+      xrealloc(saved_lengths, psize * argcount);
       total_length += len;
     }
   va_end (args);
@@ -393,7 +394,7 @@ concat_strings (const char *str0, ...)
     }
   va_end (args);
   *p = '\0';
-
+  free(saved_lengths);
   return ret;
 }
 ^L

Reply via email to