Subject: Re: [PATCH v3 05/10] trailer: make trailer_info struct private

From: Junio C Hamano <[email-masked]>
Date: Fri, 02 Feb 2024 20:43:06 -0800

Linus Arver <[email-masked]> writes:

>>> +size_t trailer_block_start(struct trailer_info *info);
>>> +size_t trailer_block_end(struct trailer_info *info);
>>> +int blank_line_before_trailer_block(struct trailer_info *info);
>>
>> And we need new accessors, which is a good change regardless of the
>> answer to the "do we really want an extra pointer dereference?
>> Shouldn't the existing 'private' and 'internal' label we see below
>> sufficient?" question.
>
> I am very surprised with your response here, because I was expecting the
> complete opposite reaction from reviewers --- something like
>
>     Hmph, we have to write accessor functions now when before we could
>     just reach inside the struct directly? Isn't this just adding
>     needless verbosity?
>
> (perhaps with more dissatisfaction). Something tells me you meant "bad
> change" but accidentally wrote "good change". Am I correct?

I often make typoes and screw up my double negations, but not this
time.  While I still suspect that the extra secrecy afforded by
using a pointer to an opaque structure is something unnecessary
between friends, as coding convention, it would be a good change to
introduce accessors and have them used by the API users, i.e. code
outside the API implementation.

You can still "git grep" for references to the ".trailer_private"
(or whatever the "private" members are named by convention) member
outside the trailer "client code" to see if there are people who are
too intimate than they should be that way, as they should all be
using the published accessors.