Re: [PATCH 1/3] set errno=0 before strtoX calls

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1634 bytes --]

On Fri, Aug 02, 2024 at 04:10:51AM +0000, Kyle Lippincott via GitGitGadget wrote:
> From: Kyle Lippincott <spectral@google.com>
> 
> To detect conversion failure after calls to functions like `strtod`, one
> can check `errno == ERANGE`. These functions are not guaranteed to set
> `errno` to `0` on successful conversion, however. Manual manipulation of
> `errno` can likely be avoided by checking that the output pointer
> differs from the input pointer, but that's not how other locations, such
> as parse.c:139, handle this issue; they set errno to 0 prior to
> executing the function.
> 
> For every place I could find a strtoX function with an ERANGE check
> following it, set `errno = 0;` prior to executing the conversion
> function.

Makes sense. I've also gone through callsites and couldn't spot any
additional ones that are broken.

Generally speaking, the interfaces provided by the `strtod()` family of
functions is just plain awful, and ideally we wouldn't be using them in
the Git codebase at all without a wrapper. We already do have wrappers
for a subset of those functions, e.g. `strtol_i()`, which use an out
pointer to store the result and indicate success via the return value
instead of via `errno`.

It would be great if we could extend those wrappers to cover all of the
integer types, convert our code base to use them, and then extend our
"banned.h" banner. I'm of course not asking you to do that in this patch
series.

Out of curiosity, why do you hit those errors in your test setup? Do you
use a special libc that behaves differently than the most common ones?

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]