Re: [PATCH 2/9] xdiff: make xrecord_t.ptr a uint8_t instead of char

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 21, 2025, 18:15 UTC
Message-ID: <xmqqplagunnm.fsf@gitster.g>
In-Reply-To: <786d6c19-0a13-4e55-8f4b-39b57dd6ea28@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 5 quoted lines

> It C "char" never refers to a unicode code point so I don't follow the 
> reasoning here. Isn't the reason you want to change from "char" to 
> "uint8_t" to match rust? Given "char" and "uint8_t" are the same width 
> why can't we use "char" in the C struct and "u8" in the rust struct as 
> the two structs would still have the same layout?

And forcing u8 makes sure both sides of the ffi agrees on the signedness (C "char"'s signedness is implementation defined), which is a good thing.

I 100% agree that being honest about the motivation to sell this change would be a good thing to do here. I do not think "in this series, I want to match the types used at the interface to be of Rust's" is a position to be ashamed of ;-)

> I agree with Patrick's comments on this patch - it would be nice to know 
> how you decided where to add casts. Given that rust is going to be 
> optional for at least a year we should take care to leave the C code in 
> good shape with a minimum number of casts.

Thanks.