Use of field flags in struct object

Use of field flags in struct object

[Nguyen Thai Ngoc Duy]

Hi,

The "flags" field in struct object (object.h) is used in a lot of
places. The thing is, the bit definition of this field is distributed
across files (16 according to my "object.flags |=" grep). This makes
it quite easy for one function to misunderstand a bit set by another
function, for a different purpose. For instance, in upload-pack.c,
there is a comment:

/* bits #0..7 in revision.h, #8..10 in commit.c */

which is no longer true as commit.c has moved on and used another set
of bits (16..19). Luckily, commit.c seems to clean its flags. But
there's a chance we will get bit definition overlap again.

Should all of those bit definitions be gathered in one place instead?
-- 
Duy

Re: Use of field flags in struct object

[Junio C Hamano]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> The "flags" field in struct object (object.h) is used in a lot of
> places. The thing is, the bit definition of this field is distributed
> across files (16 according to my "object.flags |=" grep). This makes
> it quite easy for one function to misunderstand a bit set by another
> function, for a different purpose. For instance, in upload-pack.c,
> there is a comment:
>
> /* bits #0..7 in revision.h, #8..10 in commit.c */
>
> which is no longer true as commit.c has moved on and used another set
> of bits (16..19). Luckily, commit.c seems to clean its flags. But
> there's a chance we will get bit definition overlap again.
>
> Should all of those bit definitions be gathered in one place instead?

In the medium term, probably yes.  In longer term, we would probably need
to rethink how these flag bits are managed.  Some users of these bits
might want to be migrated to the decorate API, for example.  It may also
be a possible improvement to move some bits that are only relevant to
commit objects (e.g.  left-right traversal, treesame, boundary) to struct
commit and additionally widen the bitfields there.