On Mon, Jul 06, 2026 at 04:44:14PM +0200, Alejandro Colomar via Mutt-dev wrote:
Hi Kevin,
Thanks for the review, Alex. I should have mentioned this was mostly a refactor, with code pulled from the existing creation date field.
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?
The field according to the GPGME doc is a unsigned long: https://www.gnupg.org/documentation/manuals/gpgme/Key-objects.html unsigned long int timestampThis is the creation timestamp of the subkey. This is (unsigned long)(-1) if the timestamp is invalid, and 0 if it is not available. Note that an invalid timestamp indicates a bug in the engine.
unsigned long int expiresThis is the expiration timestamp of the subkey, or 0 if the subkey does not expire.
+ + 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.
I agree. As you noticed in this case, I was moving very old code around. We can certainly rewrite it, but I'd prefer to do that in a separate commit.
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?
No, this was a goofup on my part. I failed to notice that it was resetting p so that it could print a readable error message: bail: muttdbg(5, "invalid number: '%s'", p); return NULL; I will rework the function to that it passes p by reference and updates it on error.
+ 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?
Yeah this was also because of the refactor, trying the keep the code the exact same. I'll change this too.
+} + 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>
-- Kevin J. McCarthy GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA
signature.asc
Description: PGP signature
