Re: [PATCH RFC v4 5/9] varint: use explicit width for integers

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 10, 2025, 21:04 UTC
Message-ID: <xmqqv7lqqat0.fsf@gitster.g>
In-Reply-To: <20250910-b4-pks-rust-breaking-change-v4-5-4a63fc69278d@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 20 quoted lines

> The varint subsystem currently uses implcit widths for integers. On the
> one hand we use `uintmax_t` for the actual value. On the other hand, we
> use `int` for the length of the encoded varint.
>
> Both of these have known maximum vaules, as we only support at most 16
> bytes when encoding varints. Thus, we know that we won't ever exceed
> `uint64_t` for the actual value and `uint8_t` for the prefix length.
>
> Refactor the code to use explicit widths. Besides making the logic
> platform-independent, it also makes our life a bit easier in the next
> commit, where we reimplement "varint.c" in Rust.
>
> Suggested-by: Ezekiel Newren <ezekielnewren@gmail.com>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  dir.c        | 18 ++++++++++--------
>  read-cache.c |  6 ++++--
>  varint.c     |  6 +++---
>  varint.h     |  4 ++--
>  4 files changed, 19 insertions(+), 15 deletions(-)

...

> -int encode_varint(uintmax_t, unsigned char *);
> -uintmax_t decode_varint(const unsigned char **);
> +uint8_t encode_varint(uint64_t, unsigned char *);
> +uint64_t decode_varint(const unsigned char **);

OK. I do not think there is a reason why we MUST use u8, even though in practice 255 bytes is plenty for any "integer" that varint would want to express, so I'll let it go.

I have no objection to uint64_t side of the equation. We would not be using 128-bit integer to express sizes of object representation in the packfiles anyway.

When this series meets Ezekiel's series, I would imagine that there will be "which one between uint64_t and u64 should we use" discussion. I'd prefer uint64_t as that is what is used in the part of the code that are not yet told about the other parts moving to Rust. Consistency throughout the codebase is good.