Re: [PATCH v3 2/8] diff: convert flags to be stored in bitfields

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 1, 2017, 01:26 UTC
Message-ID: <xmqq8tfqyejq.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kajX=FmGacDSv_dDzppE6WBHGqgyCpaG1zDyENcsDbwZg@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

> After some quick research our coding style on bit fields is twofold:
> Most older code is this way and more recent code seems to prefer
>
>     unsigned <FLAGNAME> SP : SP ;

Yes, we are very inconsistent. What does the clang format rules Brandon came up with have to say on this? FWIW, checkpatch.pl is unhappy without spaces on both side.

Show 13 quoted lines

>> +static inline void diff_flags_or(struct diff_flags *a,
>> +                                const struct diff_flags *b)
>> +{
>> +       char *tmp_a = (char *)a;
>> +       const char *tmp_b = (const char *)b;
>> +       int i;
>> +
>> +       for (i = 0; i < sizeof(struct diff_flags); i++)
>
> I think most of the code prefers to put the variable into the sizeof
> argument i.e. 'sizeof(*a)', as that is presumably more maintainable.
> (If the type of 'a' changes, then we don't forget to adapt this place,
> but the compiler can take care of it.

Yup, but in this case we won't change the type, no?