Thread (5 messages) 5 messages, 3 authors, 2024-02-27

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

From: Kyle Lippincott <hidden>
Date: 2024-02-27 00:56:46

On Mon, Feb 26, 2024 at 4:20 PM Junio C Hamano [off-list ref] wrote:
Kyle Lippincott [off-list ref] writes:
quoted
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.
quoted
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,
It has. We use PRIuMAX, which is from inttypes.h. I think it's only
"accidentally" working if anyone uses NO_INTTYPES_H. I changed my
stance halfway through this investigation in my previous email, I
apologize for not going back and editing it to make it clear at the
beginning that I'd done so. My current stance is that
<git-compat-util.h> should be either (a) including only inttypes.h
(since it includes stdint.h), or (b) including both inttypes.h and
stdint.h (in either order), just to demonstrate that we can.
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.
I disagree with this statement. We _can't_ use a magic compatibility
header file in the library interfaces, for the reasons I outlined
further below in my previous message. For those headers, the ones that
might be included by code that's not under the Git project's control,
they need to be self-contained, minimal, and maximally compatible.
Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help