Hi Kevin,

On 2026-07-06T21:41:33+0800, Kevin J. McCarthy wrote:
> Create expando %(<datefmt>) for the classic PGP and GPGME select key
> menu in $pgp_entry_format.
> 
> Add "expires" to $pgp_sort_keys methods.
> 
> Refactor the datetime parser in gnupgparse.c and apply it to both the
> creation and expiration dates in the output.
> ---
>  crypt-gpgme.c | 37 ++++++++++++++++++--
>  gnupgparse.c  | 96 +++++++++++++++++++++++++++++----------------------
>  init.h        |  3 ++
>  pgpkey.c      | 27 +++++++++++++--
>  pgplib.h      |  1 +
>  sort.h        |  1 +
>  6 files changed, 118 insertions(+), 47 deletions(-)
> 
> diff --git a/crypt-gpgme.c b/crypt-gpgme.c
> index f5ee87bd..6badab78 100644
> --- a/crypt-gpgme.c
> +++ b/crypt-gpgme.c
> @@ -3287,12 +3287,14 @@ static const char *crypt_entry_fmt(char *dest,
>    switch (ascii_tolower(op))
>      {
>      case '[':
> +    case '(':
>        {
>          const char *cp;
>          char buf2[SHORT_STRING], *p;
>          int do_locales;
>          struct tm *tm;
>          size_t len;
> +        int expires = (op == '(');
>  
>          p = dest;
>  
> @@ -3306,7 +3308,7 @@ static const char *crypt_entry_fmt(char *dest,
>            do_locales = 1;
>  
>          len = destlen - 1;
> -        while (len > 0 && *cp != ']')
> +        while (len > 0 && *cp != (expires ? ')' : ']'))
>          {
>            if (*cp == '%')
>            {
> @@ -3332,7 +3334,9 @@ static const char *crypt_entry_fmt(char *dest,
>          {
>            time_t tt = 0;
>  
> -          if (key->kobj->subkeys && (key->kobj->subkeys->timestamp > 0))
> +          if (expires && key->kobj->subkeys && (key->kobj->subkeys->expires 
> > 0))
> +            tt = key->kobj->subkeys->expires;
> +          else if (!expires && key->kobj->subkeys && 
> (key->kobj->subkeys->timestamp > 0))
>              tt = key->kobj->subkeys->timestamp;
>  
>            tm = localtime(&tt);
> @@ -3539,6 +3543,32 @@ static int crypt_compare_date(const void *a, const 
> void *b)
>                                           :  _crypt_compare_date(a, b));
>  }
>  
> +/* Compare 2 creation dates and the addresses.  For sorting. */
> +static int _crypt_compare_expires(const void *a, const void *b)
> +{
> +  const crypt_key_t * const *s = (const crypt_key_t * const *) a;
> +  const crypt_key_t * const *t = (const crypt_key_t * const *) b;
> +  unsigned long ts = 0, tt = 0;

Why don't we use time_t?  Can it overflow?

> +
> +  if ((*s)->kobj->subkeys && ((*s)->kobj->subkeys->expires > 0))
> +    ts = (*s)->kobj->subkeys->expires;
> +  if ((*t)->kobj->subkeys && ((*t)->kobj->subkeys->expires > 0))
> +    tt = (*t)->kobj->subkeys->expires;
> +
> +  if (ts > tt)
> +    return 1;
> +  if (ts < tt)
> +    return -1;
> +
> +  return mutt_strcasecmp((*s)->uid, (*t)->uid);
> +}
> +
> +static int crypt_compare_expires(const void *a, const void *b)
> +{
> +  return ((PgpSortKeys & SORT_REVERSE) ? !_crypt_compare_expires(a, b)
> +                                         :  _crypt_compare_expires(a, b));
> +}
> +
>  /* Compare two trust values, the key length, the creation dates. the
>     addresses and the key IDs.  For sorting. */
>  static int _crypt_compare_trust(const void *a, const void *b)
> @@ -4539,6 +4569,9 @@ static crypt_key_t *crypt_select_key(crypt_key_t *keys,
>      case SORT_DATE:
>        f = crypt_compare_date;
>        break;
> +    case SORT_EXPIRES:
> +      f = crypt_compare_expires;
> +      break;
>      case SORT_KEYID:
>        f = crypt_compare_keyid;
>        break;
> diff --git a/gnupgparse.c b/gnupgparse.c
> index 81b25503..f3c3d37c 100644
> --- a/gnupgparse.c
> +++ b/gnupgparse.c
> @@ -116,6 +116,53 @@ static void fix_uid(char *uid)
>    }
>  }
>  
> +static int parse_timestamp(char *p, time_t *timestamp)
> +{
> +  if (strchr(p, '-'))   /* gpg pre-2.0.10 used format (yyyy-mm-dd) */
> +  {
> +    char tstr[11];
> +    struct tm time;
> +
> +    time.tm_sec = 0;
> +    time.tm_min = 0;
> +    time.tm_hour = 12;



> +    strncpy(tstr, p, 11);
> +    tstr[4] = '\0';
> +    tstr[7] = '\0';

strncpy(3) should never be used for copying strings with truncation.

Some of its issues are:

-  It forces you to terminate explicitly, which is error-prone.
-  It doesn't detect truncation.
-  It makes it more difficult to know if the zeroing is superfluous or
   necessary.

In this case, I'd use the following to replace the code above:

        const char  *y, *m, *d;

        if (strlen(p) >= sizeof(tstr))
                goto bail;
        strcpy(tstr, p);
        p = tstr;
        y = strsep(&p, "-");
        m = strsep(&p, "-");
        d = strsep(&p, "-");
        if (p != NULL)
                goto bail;

> +    if (mutt_atoi(tstr, &time.tm_year, 0) < 0)

And then here, I'd use y (and in the next two calls, m and d).
> +    {
> +      p = tstr;

Setting p here seems dead code; am I missing something?

> +      goto bail;
> +    }
> +    time.tm_year -= 1900;
> +    if (mutt_atoi(tstr+5, &time.tm_mon, 0) < 0)
> +    {
> +      p = tstr+5;
> +      goto bail;
> +    }
> +    time.tm_mon -= 1;
> +    if (mutt_atoi(tstr+8, &time.tm_mday, 0) < 0)
> +    {
> +      p = tstr+8;
> +      goto bail;
> +    }
> +    *timestamp = mutt_mktime(&time, 0);
> +  }
> +  else                  /* gpg 2.0.10+ uses seconds since 1970-01-01 */
> +  {
> +    unsigned long long secs;
> +
> +    if (mutt_atoull(p, &secs, MUTT_ATOI_ALLOW_EMPTY) < 0)
> +      goto bail;
> +    *timestamp = (time_t)secs;
> +  }
> +
> +  return 0;
> +
> +bail:
> +  return -1;

Why not return -1 directly instead of goto?

> +}
> +
>  static pgp_key_t parse_pub_line(char *buf, int *is_subkey, pgp_key_t k)
>  {
>    pgp_uid_t *uid = NULL;
> @@ -245,50 +292,15 @@ static pgp_key_t parse_pub_line(char *buf, int 
> *is_subkey, pgp_key_t k)
>  
>        }
>        case 6:                   /* timestamp (1998-02-28) */
> -      {
>          muttdbg(2, "time stamp: %s", p);
> -
> -        if (strchr(p, '-'))   /* gpg pre-2.0.10 used format (yyyy-mm-dd) */
> -        {
> -          char tstr[11];
> -          struct tm time;
> -
> -          time.tm_sec = 0;
> -          time.tm_min = 0;
> -          time.tm_hour = 12;
> -          strncpy(tstr, p, 11);
> -          tstr[4] = '\0';
> -          tstr[7] = '\0';
> -          if (mutt_atoi(tstr, &time.tm_year, 0) < 0)
> -          {
> -            p = tstr;
> -            goto bail;

Ohhh, now I see where it comes from.  :)

It would be nice to have a second commit that cleans up and removes dead
code (if you want, I can send that).


Have a lovely day!
Alex

> -          }
> -          time.tm_year -= 1900;
> -          if (mutt_atoi(tstr+5, &time.tm_mon, 0) < 0)
> -          {
> -            p = tstr+5;
> -            goto bail;
> -          }
> -          time.tm_mon -= 1;
> -          if (mutt_atoi(tstr+8, &time.tm_mday, 0) < 0)
> -          {
> -            p = tstr+8;
> -            goto bail;
> -          }
> -          tmp.gen_time = mutt_mktime(&time, 0);
> -        }
> -        else                  /* gpg 2.0.10+ uses seconds since 1970-01-01 */
> -        {
> -          unsigned long long secs;
> -
> -          if (mutt_atoull(p, &secs, MUTT_ATOI_ALLOW_EMPTY) < 0)
> -            goto bail;
> -          tmp.gen_time = (time_t)secs;
> -        }
> +        if (parse_timestamp(p, &tmp.gen_time))
> +          goto bail;
>          break;
> -      }
> -      case 7:                   /* valid for n days */
> +
> +      case 7:                   /* expires timestamp */
> +        muttdbg(2, "expires time stamp: %s", p);
> +        if (parse_timestamp(p, &tmp.exp_time))
> +          goto bail;
>          break;
>        case 8:                   /* Local id         */
>          break;
> diff --git a/init.h b/init.h
> index af684866..087ebfa8 100644
> --- a/init.h
> +++ b/init.h
> @@ -168,6 +168,7 @@ const struct mapping_t SortAliasMethods[] = {  /* 
> DT_SORT_ALIAS */
>  const struct mapping_t SortKeyMethods[] = {  /* DT_SORT_KEYS */
>    { "address",  SORT_ADDRESS },
>    { "date",     SORT_DATE },
> +  { "expires",  SORT_EXPIRES },
>    { "keyid",    SORT_KEYID },
>    { "trust",    SORT_TRUST },
>    { NULL,       0 }
> @@ -2641,6 +2642,7 @@ struct option_t MuttVars[] = {
>    ** .dt %c     .dd capabilities
>    ** .dt %t     .dd trust/validity of the key-uid association
>    ** .dt %[<s>] .dd date of the key where <s> is an \fCstrftime(3)\fP 
> expression
> +  ** .dt %(<s>) .dd expires date of the key where <s> is an 
> \fCstrftime(3)\fP expression
>    ** .de
>    ** .pp
>    ** (PGP only)
> @@ -2834,6 +2836,7 @@ struct option_t MuttVars[] = {
>    ** .dt address .dd sort alphabetically by user id
>    ** .dt keyid   .dd sort alphabetically by key id
>    ** .dt date    .dd sort by key creation date
> +  ** .dt expires .dd sort by key expiration date
>    ** .dt trust   .dd sort by the trust of the key
>    ** .de
>    ** .pp
> diff --git a/pgpkey.c b/pgpkey.c
> index a6532e4c..06789ea3 100644
> --- a/pgpkey.c
> +++ b/pgpkey.c
> @@ -152,13 +152,14 @@ static const char *pgp_entry_fmt(char *dest,
>    switch (ascii_tolower(op))
>    {
>      case '[':
> -
> +    case '(':
>      {
>        const char *cp;
>        char buf2[SHORT_STRING], *p;
>        int do_locales;
>        struct tm *tm;
>        size_t len;
> +      int expires = (op == '(');
>  
>        p = dest;
>  
> @@ -172,7 +173,7 @@ static const char *pgp_entry_fmt(char *dest,
>          do_locales = 1;
>  
>        len = destlen - 1;
> -      while (len > 0 && *cp != ']')
> +      while (len > 0 && *cp != (expires ? ')' : ']'))
>        {
>          if (*cp == '%')
>          {
> @@ -196,7 +197,7 @@ static const char *pgp_entry_fmt(char *dest,
>        *p = 0;
>  
>  
> -      tm = localtime(&key->gen_time);
> +      tm = localtime(expires ? &key->exp_time : &key->gen_time);
>  
>        if (!do_locales)
>          setlocale(LC_TIME, "C");
> @@ -355,6 +356,23 @@ static int pgp_compare_date(const void *a, const void *b)
>            : _pgp_compare_date(a, b));
>  }
>  
> +static int _pgp_compare_expires(const void *a, const void *b)
> +{
> +  int r;
> +  const pgp_uid_t * const *s = (const pgp_uid_t * const *) a;
> +  const pgp_uid_t * const *t = (const pgp_uid_t * const *) b;
> +
> +  if ((r = mutt_numeric_cmp((*s)->parent->exp_time, (*t)->parent->exp_time)))
> +    return r;
> +  return (mutt_strcasecmp((*s)->addr, (*t)->addr));
> +}
> +
> +static int pgp_compare_expires(const void *a, const void *b)
> +{
> +  return ((PgpSortKeys & SORT_REVERSE) ? !_pgp_compare_expires(a, b)
> +          : _pgp_compare_expires(a, b));
> +}
> +
>  static int _pgp_compare_trust(const void *a, const void *b)
>  {
>    int r;
> @@ -502,6 +520,9 @@ static pgp_key_t pgp_select_key(pgp_key_t keys,
>      case SORT_DATE:
>        f = pgp_compare_date;
>        break;
> +    case SORT_EXPIRES:
> +      f = pgp_compare_expires;
> +      break;
>      case SORT_KEYID:
>        f = pgp_compare_keyid;
>        break;
> diff --git a/pgplib.h b/pgplib.h
> index 4a7fa94e..cccd8d9d 100644
> --- a/pgplib.h
> +++ b/pgplib.h
> @@ -38,6 +38,7 @@ struct pgp_keyinfo
>    int flags;
>    short keylen;
>    time_t gen_time;
> +  time_t exp_time;
>    int numalg;
>    const char *algorithm;
>    struct pgp_keyinfo *parent;
> diff --git a/sort.h b/sort.h
> index 863a6655..9fe1e557 100644
> --- a/sort.h
> +++ b/sort.h
> @@ -37,6 +37,7 @@
>  #define SORT_LABEL      19
>  #define SORT_AUX        20  /* $sort_thread_groups delegation to $sort_aux */
>  #define SORT_UID        21  /* used internally by the IMAP code */
> +#define SORT_EXPIRES    22
>  
>  /* Sort and sort_aux are shorts, and are a composite of a
>   * constant sort operation number and a set of compounded
> -- 
> 2.55.0
> 

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

Attachment: signature.asc
Description: PGP signature

Reply via email to