git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: Ezekiel Newren via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,  Ezekiel Newren <ezekielnewren@gmail.com>,
	 Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH 2/9] xdiff: make xrecord_t.ptr a uint8_t instead of char
Date: Tue, 21 Oct 2025 11:15:25 -0700	[thread overview]
Message-ID: <xmqqplagunnm.fsf@gitster.g> (raw)
In-Reply-To: <786d6c19-0a13-4e55-8f4b-39b57dd6ea28@gmail.com> (Phillip Wood's message of "Tue, 21 Oct 2025 14:13:52 +0100")

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

> 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.

  reply	other threads:[~2025-10-21 18:15 UTC|newest]

Thread overview: 118+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-15 21:18 [PATCH 0/9] Xdiff cleanup part2 Ezekiel Newren via GitGitGadget
2025-10-15 21:18 ` [PATCH 1/9] xdiff: use ssize_t for dstart/dend, make them last in xdfile_t Ezekiel Newren via GitGitGadget
2025-10-21 11:32   ` Phillip Wood
2025-10-21 17:18     ` Junio C Hamano
2025-10-22 21:07       ` Ezekiel Newren
2025-10-22 21:38         ` Junio C Hamano
2025-10-22 21:51           ` Ezekiel Newren
2025-10-15 21:18 ` [PATCH 2/9] xdiff: make xrecord_t.ptr a uint8_t instead of char Ezekiel Newren via GitGitGadget
2025-10-16 21:51   ` Kristoffer Haugsbakk
2025-10-21  8:33   ` Patrick Steinhardt
2025-10-22 21:12     ` Ezekiel Newren
2025-10-21 13:13   ` Phillip Wood
2025-10-21 18:15     ` Junio C Hamano [this message]
2025-10-22 13:27       ` Phillip Wood
2025-10-22 20:55         ` Ezekiel Newren
2025-10-15 21:18 ` [PATCH 3/9] xdiff: use size_t for xrecord_t.size Ezekiel Newren via GitGitGadget
2025-10-15 21:18 ` [PATCH 4/9] xdiff: use unambiguous types in xdl_hash_record() Ezekiel Newren via GitGitGadget
2025-10-21  8:33   ` Patrick Steinhardt
2025-10-22 21:20     ` Ezekiel Newren
2025-10-23  5:49       ` Patrick Steinhardt
2025-10-15 21:18 ` [PATCH 5/9] xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash Ezekiel Newren via GitGitGadget
2025-10-20 23:29   ` Ezekiel Newren
2025-10-21  5:10     ` Junio C Hamano
2025-10-21  8:33     ` Patrick Steinhardt
2025-10-21 10:03     ` Phillip Wood
2025-10-21 11:16       ` Chris Torek
2025-10-22 21:31       ` Ezekiel Newren
2025-10-15 21:18 ` [PATCH 6/9] xdiff: make xdfile_t.nrec a size_t instead of long Ezekiel Newren via GitGitGadget
2025-10-15 21:18 ` [PATCH 7/9] xdiff: make xdfile_t.nreff " Ezekiel Newren via GitGitGadget
2025-10-15 21:18 ` [PATCH 8/9] xdiff: change rindex from long to size_t in xdfile_t Ezekiel Newren via GitGitGadget
2025-10-21  8:34   ` Patrick Steinhardt
2025-10-22 22:14     ` Ezekiel Newren
2025-10-23  5:49       ` Patrick Steinhardt
2025-10-15 21:18 ` [PATCH 9/9] xdiff: rename rindex -> reference_index Ezekiel Newren via GitGitGadget
2025-10-15 21:28 ` [PATCH 0/9] Xdiff cleanup part2 Junio C Hamano
2025-10-21 13:28 ` Phillip Wood
2025-10-21 13:41   ` Junio C Hamano
2025-10-29 22:19 ` [PATCH v2 00/10] " Ezekiel Newren via GitGitGadget
2025-10-29 22:19   ` [PATCH v2 01/10] doc: define unambiguous type mappings across C and Rust Ezekiel Newren via GitGitGadget
2025-11-06  9:55     ` Phillip Wood
2025-11-06 22:52       ` Ezekiel Newren
2025-11-09 14:14         ` Phillip Wood
2025-10-29 22:19   ` [PATCH v2 02/10] xdiff: use ssize_t for dstart/dend, make them last in xdfile_t Ezekiel Newren via GitGitGadget
2025-11-06  9:55     ` Phillip Wood
2025-11-06 22:56       ` Ezekiel Newren
2025-10-29 22:19   ` [PATCH v2 03/10] xdiff: make xrecord_t.ptr a uint8_t instead of char Ezekiel Newren via GitGitGadget
2025-11-06 10:49     ` Phillip Wood
2025-11-06 23:13       ` Ezekiel Newren
2025-11-06 10:55     ` Phillip Wood
2025-11-06 23:14       ` Ezekiel Newren
2025-10-29 22:19   ` [PATCH v2 04/10] xdiff: use size_t for xrecord_t.size Ezekiel Newren via GitGitGadget
2025-10-29 22:19   ` [PATCH v2 05/10] xdiff: use unambiguous types in xdl_hash_record() Ezekiel Newren via GitGitGadget
2025-10-29 22:19   ` [PATCH v2 06/10] xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash Ezekiel Newren via GitGitGadget
2025-11-06 11:00     ` Phillip Wood
2025-11-06 23:20       ` Ezekiel Newren
2025-10-29 22:19   ` [PATCH v2 07/10] xdiff: make xdfile_t.nrec a size_t instead of long Ezekiel Newren via GitGitGadget
2025-10-29 22:19   ` [PATCH v2 08/10] xdiff: make xdfile_t.nreff " Ezekiel Newren via GitGitGadget
2025-10-29 22:19   ` [PATCH v2 09/10] xdiff: change rindex from long to size_t in xdfile_t Ezekiel Newren via GitGitGadget
2025-10-29 22:19   ` [PATCH v2 10/10] xdiff: rename rindex -> reference_index Ezekiel Newren via GitGitGadget
2025-10-30 14:26   ` [PATCH v2 00/10] Xdiff cleanup part2 Junio C Hamano
2025-11-11 19:42   ` [PATCH v3 " Ezekiel Newren via GitGitGadget
2025-11-11 19:42     ` [PATCH v3 01/10] doc: define unambiguous type mappings across C and Rust Ezekiel Newren via GitGitGadget
2025-11-11 20:52       ` Junio C Hamano
2025-11-11 21:05       ` Junio C Hamano
2025-11-11 19:42     ` [PATCH v3 02/10] xdiff: use ptrdiff_t for dstart/dend Ezekiel Newren via GitGitGadget
2025-11-11 22:23       ` Junio C Hamano
2025-11-11 19:42     ` [PATCH v3 03/10] xdiff: make xrecord_t.ptr a uint8_t instead of char Ezekiel Newren via GitGitGadget
2025-11-11 22:53       ` Junio C Hamano
2025-11-11 19:42     ` [PATCH v3 04/10] xdiff: use size_t for xrecord_t.size Ezekiel Newren via GitGitGadget
2025-11-11 23:08       ` Junio C Hamano
2025-11-14  6:02         ` Ezekiel Newren
2025-11-14 16:31           ` Junio C Hamano
2025-11-11 19:42     ` [PATCH v3 05/10] xdiff: use unambiguous types in xdl_hash_record() Ezekiel Newren via GitGitGadget
2025-11-11 19:42     ` [PATCH v3 06/10] xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash Ezekiel Newren via GitGitGadget
2025-11-11 23:21       ` Junio C Hamano
2025-11-14  5:41         ` Ezekiel Newren
2025-11-14 20:06           ` Junio C Hamano
2025-11-11 19:42     ` [PATCH v3 07/10] xdiff: make xdfile_t.nrec a size_t instead of long Ezekiel Newren via GitGitGadget
2025-11-11 19:42     ` [PATCH v3 08/10] xdiff: make xdfile_t.nreff " Ezekiel Newren via GitGitGadget
2025-11-11 19:42     ` [PATCH v3 09/10] xdiff: change rindex from long to size_t in xdfile_t Ezekiel Newren via GitGitGadget
2025-11-11 19:42     ` [PATCH v3 10/10] xdiff: rename rindex -> reference_index Ezekiel Newren via GitGitGadget
2025-11-11 23:40     ` [PATCH v3 00/10] Xdiff cleanup part2 Junio C Hamano
2025-11-14  5:52       ` Ezekiel Newren
2025-11-14 22:36     ` [PATCH v4 " Ezekiel Newren via GitGitGadget
2025-11-14 22:36       ` [PATCH v4 01/10] doc: define unambiguous type mappings across C and Rust Ezekiel Newren via GitGitGadget
2025-11-15  3:06         ` Ramsay Jones
2025-11-15  3:41           ` Ben Knoble
2025-11-15 14:55             ` Ramsay Jones
2025-11-15 16:42               ` Junio C Hamano
2025-11-15 16:59                 ` D. Ben Knoble
2025-11-15 20:03                   ` Junio C Hamano
2025-11-17  1:20                 ` Junio C Hamano
2025-11-17  2:08                   ` Ramsay Jones
2025-11-14 22:36       ` [PATCH v4 02/10] xdiff: use ptrdiff_t for dstart/dend Ezekiel Newren via GitGitGadget
2025-11-14 22:36       ` [PATCH v4 03/10] xdiff: make xrecord_t.ptr a uint8_t instead of char Ezekiel Newren via GitGitGadget
2025-11-15  8:26         ` Junio C Hamano
2025-11-18 20:55           ` Ezekiel Newren
2025-11-14 22:36       ` [PATCH v4 04/10] xdiff: use size_t for xrecord_t.size Ezekiel Newren via GitGitGadget
2025-11-14 22:36       ` [PATCH v4 05/10] xdiff: use unambiguous types in xdl_hash_record() Ezekiel Newren via GitGitGadget
2025-11-14 22:36       ` [PATCH v4 06/10] xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash Ezekiel Newren via GitGitGadget
2025-11-14 22:36       ` [PATCH v4 07/10] xdiff: make xdfile_t.nrec a size_t instead of long Ezekiel Newren via GitGitGadget
2025-11-14 22:36       ` [PATCH v4 08/10] xdiff: make xdfile_t.nreff " Ezekiel Newren via GitGitGadget
2025-11-14 22:36       ` [PATCH v4 09/10] xdiff: change rindex from long to size_t in xdfile_t Ezekiel Newren via GitGitGadget
2025-11-14 22:36       ` [PATCH v4 10/10] xdiff: rename rindex -> reference_index Ezekiel Newren via GitGitGadget
2025-11-18 22:34       ` [PATCH v5 00/10] Xdiff cleanup part2 Ezekiel Newren via GitGitGadget
2025-11-18 22:34         ` [PATCH v5 01/10] doc: define unambiguous type mappings across C and Rust Ezekiel Newren via GitGitGadget
2025-11-18 23:46           ` Ramsay Jones
2025-11-19  4:14             ` Junio C Hamano
2025-11-18 22:34         ` [PATCH v5 02/10] xdiff: use ptrdiff_t for dstart/dend Ezekiel Newren via GitGitGadget
2025-11-18 22:34         ` [PATCH v5 03/10] xdiff: make xrecord_t.ptr a uint8_t instead of char Ezekiel Newren via GitGitGadget
2025-11-18 22:34         ` [PATCH v5 04/10] xdiff: use size_t for xrecord_t.size Ezekiel Newren via GitGitGadget
2025-11-18 22:34         ` [PATCH v5 05/10] xdiff: use unambiguous types in xdl_hash_record() Ezekiel Newren via GitGitGadget
2025-11-18 22:34         ` [PATCH v5 06/10] xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash Ezekiel Newren via GitGitGadget
2025-11-18 22:34         ` [PATCH v5 07/10] xdiff: make xdfile_t.nrec a size_t instead of long Ezekiel Newren via GitGitGadget
2025-11-18 22:34         ` [PATCH v5 08/10] xdiff: make xdfile_t.nreff " Ezekiel Newren via GitGitGadget
2025-11-18 22:34         ` [PATCH v5 09/10] xdiff: change rindex from long to size_t in xdfile_t Ezekiel Newren via GitGitGadget
2025-11-18 22:34         ` [PATCH v5 10/10] xdiff: rename rindex -> reference_index Ezekiel Newren via GitGitGadget
2025-11-18 23:11         ` [PATCH v5 00/10] Xdiff cleanup part2 Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqplagunnm.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=ezekielnewren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=phillip.wood123@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).