Re: [PATCH 1/5] trailer: separate public from internal portion of trailer_iterator

From: Linus Arver <linusa@google.com>
Date: Aug 10, 2023, 23:15 UTC
Message-ID: <owlyy1iifq0n.fsf@fine.c.googlers.com>
In-Reply-To: <1fd1f22d-e0db-04f3-7235-899b10909c7a@gmail.com>

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

> We have something similar in unpack_trees.h see 576de3d9560 
> (unpack_trees: start splitting internal fields from public API, 
> 2023-02-27). That adds an "internal" member to "sturct unpack_trees" of 
> type "struct unpack_trees_internal which seems to be a easier naming scheme.

Ack, I will use "internal" as the member name in the next reroll.

>>> +#define private __private_to_trailer_c__do_not_use
> [...]
> That #define is pretty ugly

Haha, indeed. But I think that's the point though (i.e., the degree of ugliness matches the strength of our codebase's posture to discourage its use by external users).

I will drop the #define in the next reroll though, so, I guess it's a moot point anyway.

Show 5 quoted lines

> Another common scheme is to have an opaque pointer to the private struct 
>   in the public struct (aka pimpl idiom). The merge machinery uses this 
> - see merge-recursive.h. (I'm working on something similar for the 
> sequencer so we can change the internals without having to re-compile 
> everything that includes "sequencer.h")

Very interesting! I look forward to seeing your work. :)

Show 5 quoted lines

>> - Prefixing private members with "__" (khash.h and other header-only
>>    libraries use this at least, not sure if we have this in the 'main
>>    tree'). I think this works pretty well most of the time.
>
> It is common but I think the C standard reserves names beginning with "__"

Indeed (see [1]).

[1] https://devblogs.microsoft.com/oldnewthing/20230109-00/?p=107685