Re: [PATCH v3] credential: new attribute password_expiry_utc

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 17, 2023, 21:59 UTC
Message-ID: <xmqqa61ckl6j.fsf@gitster.g>
In-Reply-To: <85ab572a-cd00-f62a-97ab-f344e2b6f68e@gmail.com>

Lessley Dennington <lessleydennington@gmail.com> writes:

Show 12 quoted lines

> diff --git a/credential.c b/credential.c
> index d3e1bf7a67..b9a9a1d7b1 100644
> --- a/credential.c
> +++ b/credential.c
> @@ -236,6 +236,7 @@ int credential_read(struct credential *c, FILE *fp)
>                         free(c->path);
>                         c->path = xstrdup(value);
>                 } else if (!strcmp(key, "password_expiry_utc")) {
> +                       errno = 0;
>                         c->password_expiry_utc = parse_timestamp(value, NULL, 10);
>                         if (c->password_expiry_utc == 0 || errno)
>                                 c->password_expiry_utc = TIME_MAX;

Ah, that is quite understandable. Successful library function calls would not _clera_ errno, so if there were a failure before the control reaches this codepath, errno may have been set, and then parse_timestamp() call, which would be a strto$some_integral_type() call, may succeed and will leave errno as-is. Your fix is absolutely correct as long as we want to use "errno" after the call returns.

When strtoumax() etc. wants to report overflow or underflow, the returned value must be UINTMAX_MAX/UINTMAX_MIN and errno would be ERANGE, so it would probably want to check that errno is that value, and/or what c->password_expiry_utc has these overflow/underflow values.

> ... I have not confirmed on freebsd,
> though, as a heads up.