Add tests for mutt_addrlist_write + make it more readable

diff --git a/address/address.c b/address/address.c
index fb4352a658221b7c5693d218c25f68928eca7563..2a59fad068f373e6b28f4ad412540dcbcc2744e4 100644 (file)
--- a/address/address.c
+++ b/address/address.c
@@ -1006,14 +1006,14 @@ const char *mutt_addr_for_display(const struct Address *a)
  * @param buflen  Length of the buffer
  * @param addr    Address to display
  * @param display This address will be displayed to the user
- * @retval num    Number of characters written to buf
+ * @retval num    Length of the string written to buf
  *
  * If 'display' is set, then it doesn't matter if the transformation isn't
  * reversible.
  */
 size_t mutt_addr_write(char *buf, size_t buflen, struct Address *addr, bool display)
 {
-  if (!buf || !addr)
+  if (!buf || buflen == 0 || !addr)
     return 0;
 
   size_t len;
@@ -1051,8 +1051,7 @@ size_t mutt_addr_write(char *buf, size_t buflen, struct Address *addr, bool disp
     {
       if (buflen == 0)
         goto done;
-      mutt_str_strfcpy(pbuf, addr->personal, buflen);
-      len = mutt_str_strlen(pbuf);
+      len = mutt_str_strfcpy(pbuf, addr->personal, buflen + 1 /* strfcpy terminates */);
       pbuf += len;
       buflen -= len;
     }
@@ -1078,8 +1077,7 @@ size_t mutt_addr_write(char *buf, size_t buflen, struct Address *addr, bool disp
     if (mutt_str_strcmp(addr->mailbox, "@") != 0)
     {
       const char *a = display ? mutt_addr_for_display(addr) : addr->mailbox;
-      mutt_str_strfcpy(pbuf, a, buflen);
-      len = mutt_str_strlen(pbuf);
+      len = mutt_str_strfcpy(pbuf, a, buflen + 1 /* strfcpy terminates */);
       pbuf += len;
       buflen -= len;
     }
@@ -1128,7 +1126,7 @@ done:
  * @param buflen  Length of the buffer
  * @param al      AddressList to display
  * @param display This address will be displayed to the user
- * @retval num Bytes written to the buffer
+ * @retval num    Length of the string written to buf
  *
  * If 'display' is set, then it doesn't matter if the transformation isn't
  * reversible.
@@ -1137,64 +1135,45 @@ done:
  */
 size_t mutt_addrlist_write(char *buf, size_t buflen, const struct AddressList *al, bool display)
 {
-  if (!buf || !al)
+  if (!buf || buflen == 0 || !al)
     return 0;
 
-  char *pbuf = buf;
   size_t len = mutt_str_strlen(buf);
-
-  buflen--; /* save room for the terminal nul */
-
-  if (len > 0)
+  if (len >= buflen)
   {
-    if (len > buflen)
-      return 0; /* safety check for bogus arguments */
-
-    pbuf += len;
-    buflen -= len;
-    if (buflen == 0)
-      goto done;
-    *pbuf++ = ',';
-    buflen--;
-    if (buflen == 0)
-      goto done;
-    *pbuf++ = ' ';
-    buflen--;
+    return 0;
   }
 
+  char *pbuf = buf + len;
+  buflen -= len;
+
   struct Address *a = NULL;
   TAILQ_FOREACH(a, al, entries)
   {
-    if (buflen == 0)
-      break;
+    if (len > 0)
+    {
+      if (buflen > 1)
+      {
+        *pbuf++ = ',';
+        buflen--;
+      }
+      if (buflen > 1)
+      {
+        *pbuf++ = ' ';
+        buflen--;
+      }
+    }
 
-    /* use buflen+1 here because we already saved space for the trailing
-     * nul char, and the subroutine can make use of it */
-    mutt_addr_write(pbuf, buflen + 1, a, display);
+    if (buflen == 1)
+    {
+      break;
+    }
 
-    /* this should be safe since we always have at least 1 char passed into
-     * the above call, which means 'pbuf' should always be nul terminated */
-    len = mutt_str_strlen(pbuf);
+    len = mutt_addr_write(pbuf, buflen, a, display);
     pbuf += len;
     buflen -= len;
-
-    /* if there is another address, and it's not a group mailbox name or
-     * group terminator, add a comma to separate the addresses */
-    struct Address *next = TAILQ_NEXT(a, entries);
-    if (next && next->mailbox && !next->group)
-    {
-      if (buflen == 0)
-        goto done;
-      *pbuf++ = ',';
-      buflen--;
-      if (buflen == 0)
-        goto done;
-      *pbuf++ = ' ';
-      buflen--;
-    }
   }
 
-done:
   *pbuf = '\0';
   return pbuf - buf;
 }

diff --git a/test/address/mutt_addrlist_write.c b/test/address/mutt_addrlist_write.c
index 6bf5dcd8d3d4377c9365ffa3197cdab6abe1d9ee..50c59823e53eae75dcdb94e6e03db6ed7ef5a733 100644 (file)
--- a/test/address/mutt_addrlist_write.c
+++ b/test/address/mutt_addrlist_write.c
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "mutt/mutt.h"
 #include "address/lib.h"
+#include "common.h"
 
 void test_mutt_addrlist_write(void)
 {
@@ -33,13 +34,48 @@ void test_mutt_addrlist_write(void)
 
   {
     struct AddressList al = { 0 };
-    mutt_addrlist_write(NULL, 32, &al, false);
-    TEST_CHECK_(1, "mutt_addrlist_write(NULL, 32, &al, false)");
+    TEST_CHECK(mutt_addrlist_write(NULL, 32, &al, false) == 0);
   }
 
   {
     char buf[32] = { 0 };
-    mutt_addrlist_write(buf, sizeof(buf), NULL, false);
-    TEST_CHECK_(1, "mutt_addrlist_write(buf, sizeof(buf), NULL, false)");
+    TEST_CHECK(mutt_addrlist_write(buf, sizeof(buf), NULL, false) == 0);
+  }
+
+  {
+    struct AddressList al = TAILQ_HEAD_INITIALIZER(al);
+    const char *in = "test@example.com, John Doe <john@doe.org>, \"Foo J. Bar\" <foo-j-bar@baz.com>";
+    const size_t inlen = strlen(in);
+    int parsed = mutt_addrlist_parse(&al, in);
+    TEST_CHECK(parsed == 3);
+
+    {
+      char buf[8] = { 0 };
+      size_t nbytes = mutt_addrlist_write(buf, sizeof(buf), &al, false);
+      TEST_CHECK(nbytes == sizeof(buf) - 1);
+      TEST_CHECK_STR_EQ("test@ex", buf);
+    }
+
+    {
+      char buf[24] = { 0 };
+      size_t nbytes = mutt_addrlist_write(buf, sizeof(buf), &al, false);
+      TEST_CHECK(nbytes == sizeof(buf) - 1);
+      TEST_CHECK_STR_EQ("test@example.com, John ", buf);
+    }
+
+    {
+      char buf[43] = { 0 };
+      size_t nbytes = mutt_addrlist_write(buf, sizeof(buf), &al, false);
+      TEST_CHECK(nbytes == sizeof(buf) - 1);
+      TEST_CHECK_STR_EQ("test@example.com, John Doe <john@doe.org>,", buf);
+    }
+
+    {
+      char buf[76] = { 0 };
+      size_t nbytes = mutt_addrlist_write(buf, sizeof(buf), &al, false);
+      TEST_CHECK(nbytes == sizeof(buf) - 1);
+      TEST_CHECK_STR_EQ("test@example.com, John Doe <john@doe.org>, \"Foo J. Bar\" <foo-j-bar@baz.com>", buf);
+      TEST_CHECK(buf[inlen] == '\0');
+    }
   }
 }