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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 22, 2024 at 9:51 AM Calvin Wan <calvinwan@xxxxxxxxxx> wrote:
>
> From: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
>
> pager.h uses uintmax_t but does not include stdint.h. Therefore, add
> this include statement.
>
> This was discovered when writing a stub pager.c file.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
> Signed-off-by: Calvin Wan <calvinwan@xxxxxxxxxx>
> ---
>  pager.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/pager.h b/pager.h
> index b77433026d..015bca95e3 100644
> --- a/pager.h
> +++ b/pager.h
> @@ -1,6 +1,8 @@
>  #ifndef PAGER_H
>  #define PAGER_H
>
> +#include <stdint.h>
> +
>  struct child_process;
>
>  const char *git_pager(int stdout_is_tty);
> --
> 2.44.0.rc0.258.g7320e95886-goog
>
>

As far as I can tell, we need pager.h because of the `pager_in_use`
symbol. We need that symbol because of its use in date.c's
`parse_date_format`. I wonder if we can side step the `#include
<stdint.h>` concerns by splitting pager.h into pager.h and
pager_in_use.h, and have pager.h include pager_in_use.h instead. This
way pager.h (and its [unused] forward declarations) aren't part of
git-std-lib at all. I believe this was done for things like hex-ll.h,
so maybe we call it pager-ll.h. The goal being to (a) not need the
`#include <stdint.h>` because that's currently contentious, but also
(b) to identify the minimum set of symbols needed for the stubs
library, and not declare things that we don't have any intention of
actually providing / stubbing out.

I have some more thoughts on this, but they're much more appropriate
for the next patch in the series, so I'll leave them there.





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux