From: "René Scharfe" <l.s.r@web.de>
To: phillip.wood@dunelm.org.uk, Ezekiel Newren <ezekielnewren@gmail.com>
Cc: Ezekiel Newren via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH 01/10] ivec: introduce the C side of ivec
Date: Fri, 16 Jan 2026 21:19:01 +0100 [thread overview]
Message-ID: <fc291b3a-5ee5-4488-9b01-d3de32f7c257@web.de> (raw)
In-Reply-To: <c2d9a432-0753-4786-8de9-c3dcfe69ac36@gmail.com>
On 1/16/26 11:39 AM, Phillip Wood wrote:
> I've Cc'd Peff and René for a second opinion if you have time please.
>
> On 15/01/2026 15:55, Ezekiel Newren wrote:
>> On Thu, Jan 8, 2026 at 7:34 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>>>> +static void _set_capacity(void *self_, size_t new_capacity)
>>>> +{
>>>> + struct IVec_c_void *self = self_;
>>>
>>> Passing any of the ivec variants defined below to this function invokes
>>> undefined behavior because we're not casting the pointer back to the
>>> orginal type. However I think on the platforms we care about
>>> sizeof(void*) == sizeof(T*) for all T so maybe we can look the other way.
>>
>> If someone finds that this code does not work because of this
>> assumption I'd like to know. But I can't fathom a case where it
>> wouldn't work.
>
> So we have two different structs
>
> struct IVec_c_void {
> void *ptr;
> size_t length;
> size_t capacity;
> size_t element_size;
> }
>
> and
>
> struct Ivec_u8 {
> uint8_t *ptr;
> size_t length;
> size_t capacity;
> size_t element_size;
> }
>
> One the platforms we care about they will have the same memory
> layout as all pointers have the same representation. However I don't
> think they are "compatible types" in the language of the C standard
> because the type of the "ptr" member differs. That means casting
> IVec_u8* to IVec_c_void* either directly or via void* is undefined
> and so
>
> struct IVec_u8 vec;
> ivec_init(&vec, sizeof(*vec.ptr));
>
> is undefined. For the compiler to see the undefined cast it needs to
> look across translation units because the implementation of
> ivec_init() will be in a separate file to where it is called. Maybe
> that and the fact they have the same memory layout saves us from
> having to worry too much though I'm always nervous of undefined
> behavior.
True. The GCC docs give a fun example of what a compiler might do
when using different struct types to access the same memory:
https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Aliasing-Type-Rules.html
Not sure it applies to this case, but the point is that compilers
can and will do terrifying things when they smell UB, with little
concern for safety or original intent.
> An alternative would be to pass the individual struct members as function parameters
>
> void ivec_init(void **vec, size_t &length, size_t &capacity,
> size_t &element_size_, size_t element_size)
> {
> *vec = NULL;
> *length = 0;
> *capacity = 0;
> *element_size_ = element_size;
> }
The ampersands (&) should be asterisks (*), right?
> and have DEFINE_IVEC_TYPE create typesafe wrappers
>
> static inline void ivec_u8_init(struct IVec_u8 *vec)
> {
> void *ptr = vec->ptr;
> ivec_init(&ptr, &v->length, &v->capacity,
> &v->element_size, sizeof(*(v->ptr));
> vec->ptr = ptr;
> }
Mixes "v" and "vec", misses a closing parenthesis. Looks viable,
though, and this method should be applicable to the rest of the
functions as well (on the C side).
I guess this doesn't require an element_size member anymore as
each wrapper can pass in the sizeof value.
> That's safe because we cast the "ptr" member to "void*" and then
> back to the original type. On the rust side the implementation of
> IVec<T> would also need to split out the individual struct members
> when it calls ivec_init() etc. It's all a bit more effort but the
> benefit is that we don't have any undefined behavior and we have a
> nice typesafe C interface to 'struct IVec_*'.
Right. No idea how ugly this would be on the Rust side, though.
René
next prev parent reply other threads:[~2026-01-16 20:19 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-02 18:52 [PATCH 00/10] Xdiff cleanup part 3 Ezekiel Newren via GitGitGadget
2026-01-02 18:52 ` [PATCH 01/10] ivec: introduce the C side of ivec Ezekiel Newren via GitGitGadget
2026-01-04 5:32 ` Junio C Hamano
2026-01-17 16:06 ` Ezekiel Newren
2026-01-08 14:34 ` Phillip Wood
2026-01-15 15:55 ` Ezekiel Newren
2026-01-16 10:39 ` Phillip Wood
2026-01-16 20:19 ` René Scharfe [this message]
2026-01-17 13:55 ` Phillip Wood
2026-01-17 16:04 ` Ezekiel Newren
2026-01-18 14:58 ` René Scharfe
2026-01-17 16:14 ` Ezekiel Newren
2026-01-17 16:16 ` Ezekiel Newren
2026-01-17 17:40 ` Phillip Wood
2026-01-19 5:59 ` Jeff King
2026-01-19 20:21 ` Ezekiel Newren
2026-01-19 20:40 ` Jeff King
2026-01-20 2:36 ` D. Ben Knoble
2026-01-21 21:00 ` Ezekiel Newren
2026-01-21 21:20 ` Jeff King
2026-01-21 21:31 ` Junio C Hamano
2026-01-21 21:45 ` Ezekiel Newren
2026-01-20 13:46 ` Phillip Wood
2026-01-20 14:06 ` Phillip Wood
2026-01-21 21:39 ` Ezekiel Newren
2026-01-28 11:15 ` Phillip Wood
2026-01-16 20:19 ` René Scharfe
2026-01-17 15:58 ` Ezekiel Newren
2026-01-18 14:55 ` René Scharfe
2026-01-02 18:52 ` [PATCH 02/10] xdiff: make classic diff explicit by creating xdl_do_classic_diff() Ezekiel Newren via GitGitGadget
2026-01-20 15:01 ` Phillip Wood
2026-01-21 21:05 ` Ezekiel Newren
2026-01-02 18:52 ` [PATCH 03/10] xdiff: don't waste time guessing the number of lines Ezekiel Newren via GitGitGadget
2026-01-20 15:02 ` Phillip Wood
2026-01-21 21:12 ` Ezekiel Newren
2026-01-22 10:16 ` Phillip Wood
2026-01-02 18:52 ` [PATCH 04/10] xdiff: let patience and histogram benefit from xdl_trim_ends() Ezekiel Newren via GitGitGadget
2026-01-20 15:02 ` Phillip Wood
2026-01-21 14:49 ` Phillip Wood
2026-01-02 18:52 ` [PATCH 05/10] xdiff: use xdfenv_t in xdl_trim_ends() and xdl_cleanup_records() Ezekiel Newren via GitGitGadget
2026-01-20 16:32 ` Phillip Wood
2026-01-02 18:52 ` [PATCH 06/10] xdiff: cleanup xdl_trim_ends() Ezekiel Newren via GitGitGadget
2026-01-20 16:32 ` Phillip Wood
2026-01-02 18:52 ` [PATCH 07/10] xdiff: replace xdfile_t.dstart with xdfenv_t.delta_start Ezekiel Newren via GitGitGadget
2026-01-20 16:32 ` Phillip Wood
2026-01-28 10:51 ` Phillip Wood
2026-01-02 18:52 ` [PATCH 08/10] xdiff: replace xdfile_t.dend with xdfenv_t.delta_end Ezekiel Newren via GitGitGadget
2026-01-02 18:52 ` [PATCH 09/10] xdiff: remove dependence on xdlclassifier from xdl_cleanup_records() Ezekiel Newren via GitGitGadget
2026-01-16 20:19 ` René Scharfe
2026-01-17 16:34 ` Ezekiel Newren
2026-01-18 18:23 ` René Scharfe
2026-01-21 15:01 ` Phillip Wood
2026-01-02 18:52 ` [PATCH 10/10] xdiff: move xdl_cleanup_records() from xprepare.c to xdiffi.c Ezekiel Newren via GitGitGadget
2026-01-21 15:01 ` Phillip Wood
2026-01-28 10:56 ` Phillip Wood
2026-01-04 2:44 ` [PATCH 00/10] Xdiff cleanup part 3 Junio C Hamano
2026-01-04 6:01 ` Yee Cheng Chin
2026-01-28 14:40 ` Phillip Wood
2026-03-06 23:03 ` Junio C Hamano
2026-03-09 19:06 ` Ezekiel Newren
2026-03-09 23:31 ` Junio C Hamano
2026-03-25 21:11 ` [PATCH v2 0/5] " Ezekiel Newren via GitGitGadget
2026-03-25 21:11 ` [PATCH v2 1/5] xdiff/xdl_cleanup_records: delete local recs pointer Ezekiel Newren via GitGitGadget
2026-03-25 21:11 ` [PATCH v2 2/5] xdiff/xdl_cleanup_records: make limits more clear Ezekiel Newren via GitGitGadget
2026-03-25 21:11 ` [PATCH v2 3/5] xdiff/xdl_cleanup_records: make setting action easier to follow Ezekiel Newren via GitGitGadget
2026-03-25 21:11 ` [PATCH v2 4/5] xdiff/xdl_cleanup_records: simplify INVESTIGATE handling for clarity Ezekiel Newren via GitGitGadget
2026-03-25 21:11 ` [PATCH v2 5/5] xdiff/xdl_cleanup_records: use unambiguous types Ezekiel Newren via GitGitGadget
2026-03-25 21:58 ` Junio C Hamano
2026-03-26 6:26 ` [PATCH v2 0/5] Xdiff cleanup part 3 SZEDER Gábor
2026-03-27 19:23 ` [PATCH v3 0/6] " Ezekiel Newren via GitGitGadget
2026-03-27 19:23 ` [PATCH v3 1/6] xdiff/xdl_cleanup_records: delete local recs pointer Ezekiel Newren via GitGitGadget
2026-03-27 19:23 ` [PATCH v3 2/6] xdiff: use unambiguous types in xdl_bogo_sqrt() Ezekiel Newren via GitGitGadget
2026-03-27 19:23 ` [PATCH v3 3/6] xdiff/xdl_cleanup_records: use unambiguous types Ezekiel Newren via GitGitGadget
2026-03-27 19:23 ` [PATCH v3 4/6] xdiff/xdl_cleanup_records: make limits more clear Ezekiel Newren via GitGitGadget
2026-03-27 21:09 ` Junio C Hamano
2026-03-27 23:01 ` Junio C Hamano
2026-03-27 19:23 ` [PATCH v3 5/6] xdiff/xdl_cleanup_records: make setting action easier to follow Ezekiel Newren via GitGitGadget
2026-03-27 19:23 ` [PATCH v3 6/6] xdiff/xdl_cleanup_records: simplify INVESTIGATE handling for clarity Ezekiel Newren via GitGitGadget
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=fc291b3a-5ee5-4488-9b01-d3de32f7c257@web.de \
--to=l.s.r@web.de \
--cc=ezekielnewren@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=peff@peff.net \
--cc=phillip.wood@dunelm.org.uk \
/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