On Fri, 17 Feb 2023 at 21:59, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Lessley Dennington <lessleydennington@xxxxxxxxx> 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.
>
That's a subtle one! Thanks Lessley very much for your help debugging.
I shall send a patch v4 with this and some minor changes discussed in
review club.
