Re: [PATCH] Make optional for query_command

[Kevin J. McCarthy]

On Sat, Mar 05, 2022 at 01:20:08AM +0100, Magnus Groß wrote:

To fix this, we replace strtok() with strsep() which is almost
identical, except that it does not skip over consecutive tokens,
therefore the above usecase scenario will work as expected, i.e. <long
name> will be an empty string and <other info> will be the correct
token.

Thank you for the patch.  I agree that this is a misbehavior.









diff --git a/query.c b/query.c
index 5a5c07b0..d0617c23 100644
--- a/query.c
+++ b/query.c
@@ -106,7 +106,7 @@ static QUERY *run_query (char *s, int quiet)
   int dummy = 0;
   char *msg = NULL;
   size_t msglen;
-  char *p;
+  char *tok, *nexttok;
   pid_t thepid;

@@ -128,27 +128,35 @@ static QUERY *run_query (char *s, int quiet)
   msg = mutt_read_line (msg, &msglen, fp, &dummy, 0);
   while ((buf = mutt_read_line (buf, &buflen, fp, &dummy, 0)) != NULL)
   {
-    if ((p = strtok(buf, "\t\n")))
+    tok = buf;
+    if ((nexttok = strchr (tok, '\t')))
+      *nexttok++ = 0;
+
+    if (first == NULL)
     {
-      if (first == NULL)
-      {
-       first = (QUERY *) safe_calloc (1, sizeof (QUERY));
-       cur = first;
-      }
-      else
-      {
-       cur->next = (QUERY *) safe_calloc (1, sizeof (QUERY));
-       cur = cur->next;
-      }
+      first = (QUERY *) safe_calloc (1, sizeof (QUERY));
+      cur = first;
+    }
+    else
+    {
+      cur->next = (QUERY *) safe_calloc (1, sizeof (QUERY));
+      cur = cur->next;
+    }
+
+    cur->addr = rfc822_parse_adrlist (cur->addr, tok);
+    if (nexttok)
+    {
+      tok = nexttok;
+      if ((nexttok = strchr (tok, '\t')))
+        *nexttok++ = 0;
+      cur->name = safe_strdup (tok);

-      p = strtok(NULL, "\t\n");
-      if (p)
+      if (nexttok)
       {
-       cur->name = safe_strdup (p);
-       p = strtok(NULL, "\t\n");
-       if (p)
-         cur->other = safe_strdup (p);
+        tok = nexttok;
+        if ((nexttok = strchr (tok, '\t')))
+          *nexttok++ = 0;
+        cur->other = safe_strdup (tok);
       }
     }
   }


--
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA

I'd almost be willing to include it in the stable branch, except the query feature is very old, and my experience has taught me that by this point someone, somewhere, will rely on the strtok() behavior. :-/ However I do think this should be fixed in master at least, with a note in the UPDATING file describing the change in behavior. The only problem I see with the patch is that buf is allocated and reallocated by mutt_read_line(), so using strsep() will introduce a memory leak - and if a line has too many tabs, possibly a crash when FREE(&buf) is called or safe_realloc() is called in mutt_read_line(). A slightly more intrusive patch using strchr() might be better (untested, just below). This also removes '\n' from the delim list, since that's a remnant from when fgets() was used. cmd = mutt_buffer_pool_get (); - cur->addr = rfc822_parse_adrlist (cur->addr, p);

signature.asc
Description: PGP signature