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

[Stefan Beller <sbeller@google.com>]

On Tue, Oct 31, 2017 at 11:19 AM, Brandon Williams <bmwill@google.com> wrote:
> We cannot add many more flags to the diff machinery due to the
> limitations of the number of flags that can be stored in a single
> unsigned int.  In order to allow for more flags to be added to the diff
> machinery in the future this patch converts the flags to be stored in
> bitfields in 'struct diff_flags'.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>

Thanks for this cleanup series!
Stefan

> +struct diff_flags {
> +       unsigned RECURSIVE:1;

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 ;

If this turns out to be the only nit, I would ignore it for the sake of
faster settlement of the series.


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