Hi Kevin,

On 2026-01-27T20:41:59+0800, Kevin J. McCarthy wrote:
> Use the helpers extracted from is_from_forward_scan().
> 
> Create a helper to move backwards by token through the from line.
> ---
>  from.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
> 
> diff --git a/from.c b/from.c
> index 03d26c1a..f833a8e9 100644
> --- a/from.c
> +++ b/from.c
> @@ -34,6 +34,17 @@ static const char *next_word (const char *s)
>    return s;
>  }
>  
> +static const char *prev_word (const char * const bos, const char *cur)
> +{
> +  if (cur <= bos)
> +    return bos;
> +  while (cur > bos && ISSPACE (*(cur-1)))
> +    cur--;
> +  while (cur > bos && !ISSPACE (*(cur-1)))
> +    cur--;
> +  return cur;
> +}
> +
>  int mutt_check_month (const char *s)
>  {
>    int i;
> @@ -195,6 +206,86 @@ static int is_from_forward_scan (const char *s,
>    return 1;
>  }
>  
> +static int is_from_reverse_scan (const char * const bos,
> +                                 char *path, size_t pathlen,
> +                                 struct tm *tm)
> +{
> +  const char *cur = bos + strlen (bos);
> +
> +  /* year */
> +  cur = prev_word (bos, cur);
> +  if (cur == bos)
> +    return 0;
> +  if ((tm->tm_year = check_year (cur)) < 0)
> +    return 0;
> +
> +  /* timezone? */
> +  cur = prev_word (bos, cur);
> +  if (cur == bos)
> +    return 0;
> +  if (isalpha ((unsigned char) *cur) || *cur == '+' || *cur == '-')
> +  {
> +    cur = prev_word (bos, cur);
> +    if (cur == bos)
> +      return 0;
> +
> +    /*
> +     * some places have two timezone fields after the time, e.g.
> +     *      From [email protected] Wed Aug  2 00:39:12 MET DST 1995
> +     */
> +    if (isalpha ((unsigned char) *cur))
> +    {
> +      cur = prev_word (bos, cur);
> +      if (cur == bos)
> +        return 0;
> +    }
> +  }
> +
> +  /* time */
> +  if (check_time (cur, tm) < 0)
> +    return 0;
> +
> +  /* day */
> +  cur = prev_word (bos, cur);
> +  if (cur == bos)
> +    return 0;
> +  if (sscanf (cur, "%d", &tm->tm_mday) != 1)
> +    return 0;
> +
> +  /* month */
> +  cur = prev_word (bos, cur);
> +  if (cur == bos)
> +    return 0;
> +  if ((tm->tm_mon = mutt_check_month (cur)) < 0)
> +    return 0;
> +
> +  /* day name */
> +  cur = prev_word (bos, cur);
> +  if (!is_day_name (cur))
> +  {
> +    dprint(1, (debugfile,
> +               "is_from_reverse_scan():  expected weekday, got: %s\n", cur));
> +    return 0;
> +  }
> +
> +  /* return path? */
> +  while (cur > bos && ISSPACE (*(cur-1)))
> +    cur--;
> +  if (cur != bos && path)
> +  {
> +    size_t len;
> +
> +    len = (size_t) (cur - bos);
> +    if (len + 1 > pathlen)

I think calling this 'pathlen' can be misleading.  It is the size of the
buffer (sizeof), not the length of the string (strlen).  Confusing
length with size can often lead to off-by-one bugs.  I would call it
pathsize (or just size).

And then, I'd avoid the explicit +1, as it can also cause off-by-one
bugs.  A common idiom would be:

        if (len >= size)

> +      len = pathlen - 1;

Is truncation okay?  I guess in this place it's not too bad, as the
'From ' line is relatively unimportant, but just to confirm.


Have a lovely day!
Alex

> +    memcpy (path, bos, len);
> +    path[len] = 0;
> +    dprint (3, (debugfile, "is_from_reverse_scan(): got return path: %s\n", 
> path));
> +  }
> +
> +  return 1;
> +}
> +
>  /*
>   * A valid message separator looks like:
>   *
> -- 
> 2.52.0
> 

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

Attachment: signature.asc
Description: PGP signature

Reply via email to