On 2026-06-20T10:43:17+0800, Kevin J. McCarthy wrote:
> Previously, if "b" was length 127, the loop would terminate after
> copying bytes 0..125.  The final for loop increment would set i=126.
> Then, "tmp[i+1] = 0" would nul-terminate the string at byte 127.  So
> byte 126 would be unset.
> 
> Note that since none of the Capabilities mutt were passing are that
> long, the bug wouldn't matter, but it was a confusing and buggy
> function in any case.  :D
> 
> The only caller of this function isn't actually interested in a
> comparison, it's interested in whether the word "a" matches the first
> word in "b".  So rewrite the function to instead compare lengths and
> check if they are equal.  The function is much simpler and easier to
> understand that way.
> 
> Thanks to Acts1631 for the bug report.
> 
> Thanks to Alejandro Colomar for his discussion and proposed patches,
> which motivated the use of strcspn(), defining ASCII_WS, and renaming
> the function to imap_wordcaseeq().
> 
> Also thanks to Kurt Hackenberg and Ian Collier for their discussion
> and proposals, leading to the removal of the tmp[] array completely.
> ---
> 
> After thinking about it overnight, I agree with Alex that this API is
> better.  It also makes the code a tiny bit faster since it doesn't
> need to compare unless the lengths actually match.
> 
>  imap/command.c      |  2 +-
>  imap/imap_private.h |  2 +-
>  imap/util.c         | 27 +++++++++------------------
>  lib.h               |  1 +
>  4 files changed, 12 insertions(+), 20 deletions(-)
> 
> diff --git a/imap/command.c b/imap/command.c
> index 82e0ddb7..c9c4c0c2 100644
> --- a/imap/command.c
> +++ b/imap/command.c
> @@ -630,7 +630,7 @@ static void cmd_parse_capability(IMAP_DATA *idata, char 
> *s)
>    while (*s)
>    {
>      for (x = 0; x < CAPMAX; x++)
> -      if (imap_wordcasecmp(Capabilities[x], s) == 0)
> +      if (imap_wordcaseeq(Capabilities[x], s))
>        {
>          mutt_bit_set(idata->capabilities, x);
>          break;
> diff --git a/imap/imap_private.h b/imap/imap_private.h
> index f417e327..19c1f4dc 100644
> --- a/imap/imap_private.h
> +++ b/imap/imap_private.h
> @@ -328,7 +328,7 @@ void imap_quote_string_and_backquotes(char *dest, size_t 
> dlen, const char *src);
>  void imap_unquote_string(char *s);
>  void imap_munge_mbox_name(IMAP_DATA *idata, char *dest, size_t dlen, const 
> char *src);
>  void imap_unmunge_mbox_name(IMAP_DATA *idata, char *s);
> -int imap_wordcasecmp(const char *a, const char *b);
> +int imap_wordcaseeq(const char *a, const char *b);
>  SEQSET_ITERATOR *mutt_seqset_iterator_new(const char *seqset);
>  int mutt_seqset_iterator_next(SEQSET_ITERATOR *iter, unsigned int *next);
>  void mutt_seqset_iterator_free(SEQSET_ITERATOR **p_iter);
> diff --git a/imap/util.c b/imap/util.c
> index 9b6ef7b4..29f88a1b 100644
> --- a/imap/util.c
> +++ b/imap/util.c
> @@ -880,26 +880,17 @@ void imap_unmunge_mbox_name(IMAP_DATA *idata, char *s)
>    FREE(&buf);
>  }
>  
> -/* imap_wordcasecmp: find word a in word list b */
> -int imap_wordcasecmp(const char *a, const char *b)
> +/* imap_wordcaseeq:
> + *
> + * Checks if word a is case-independently equal to the initial
> + * ASCII_WS delimited word in word list b. */
> +int imap_wordcaseeq(const char *a, const char *b)
>  {
> -  char tmp[SHORT_STRING];
> -  const char *s = b;
> -  int i;
> +  size_t a_len, b_len;
>  
> -  tmp[SHORT_STRING-1] = 0;
> -  for (i=0;i < SHORT_STRING-2;i++,s++)
> -  {
> -    if (!*s || IS_ASCII_WS(*s))
> -    {
> -      tmp[i] = 0;
> -      break;
> -    }
> -    tmp[i] = *s;
> -  }
> -  tmp[i+1] = 0;
> -
> -  return ascii_strcasecmp(a, tmp);
> +  a_len = mutt_strlen(a);
> +  b_len = b ? strcspn(b, ASCII_WS) : 0;
> +  return (a_len == b_len) && !ascii_strncasecmp(a, b, a_len);

Reviewed-by: Alejandro Colomar <[email protected]>

>  }
>  
>  /*
> diff --git a/lib.h b/lib.h
> index 830459d6..72e3a498 100644
> --- a/lib.h
> +++ b/lib.h
> @@ -113,6 +113,7 @@
>   * 0x09-0x0d (\t \n \v \f \r)
>   * 0x20      (space)
>   */
> +#define ASCII_WS       " \t\n\v\f\r"
>  #define IS_ASCII_WS(c) ((9 <= (c) && (c) <= 13) || (c) == 32)
>  
>  #define SKIP_ASCII_WS(c) while (IS_ASCII_WS(*(c))) c++;
> -- 
> 2.54.0
> 

-- 
<https://www.alejandro-colomar.es>

Attachment: signature.asc
Description: PGP signature

Reply via email to