Re: [PATCH v5 1/3] pager: include stdint.h because uintmax_t is used

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 27, 2024, 00:20 UTC
Message-ID: <xmqqle76iwqw.fsf@gitster.g>
In-Reply-To: <CAO_smVi76TbmHd5w2rpBEEYbaw46SNTrekFHE-ohDC6-=dk6DA@mail.gmail.com>

Kyle Lippincott <spectral@google.com> writes:

Show 7 quoted lines

> The condition added 13 years ago was, IMHO, backwards from what it
> should have been. The intent is to have stdint.h included. We should
> include stdint.h. I suspect that 17 years is enough time for that
> platform to start conforming to what is now a 25 year old standard,
> and I don't know how we can verify that and have this stop being a
> haunted graveyard without just trying it and seeing if the build bots
> or maintainers identify it as a continuing issue.

The nightmare of Solaris might be luckily behind us, but the world does not only run Linux and GNU libc, and it is not just <stdint.h> vs <inttypes.h>. This is about general code hygiene.

> If it's still an
> issue (and only if), we should reintroduce a conditional, but invert
> it: if there's no stdint.h, THEN include inttypes.h.

But it would give the wrong order in general in the modern world, where <inttypes.h> is supposed to include <stdint.h> and extends it.

We use inttypes.h by default because the C standard already talks about it, and fall back to stdint.h when the platform lacks it. But what I suspect is that nobody compiles with NO_INTTYPES_H and we would unknowingly (but not "unintentionally") start using the extended types that are only available in <inttypes.h> but not in <stdint.h> sometime in the future. It might already have happened, but I do not know. I haven't compiled with NO_INTTYPES_H for some time (to experiment), and I haven't met a platform that actually requires NO_INTTYPES_H defined to build. Once after such a change is made without anybody being knowingly breaking some rare platform, if nobody complains, we can just drop the support to allow us to limit ourselves to <stdint.h>, but since we hear nobody complaining, we should be OK with the current rule of including system header files that is embodied in <git-compat-util.h> header file.

In any case, your sources should not include a standard library header directly yourself, period. Instead let <git-compat-util.h> take care of the details of how we need to obtain what we need out of the system on various platforms.

Thanks.