Re: [PATCH v3] credential: new attribute password_expiry_utc

[Junio C Hamano <gitster@pobox.com>]

Lessley Dennington <lessleydennington@gmail.com> writes:

> 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.