Use of field flags in struct object

Nguyen Thai Ngoc Duy· Aug 16, 2010, 08:21 UTC · lore

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

Junio C Hamano· Aug 25, 2010, 18:55 UTC · re: Nguyen Thai Ngoc Duy · lore

Re: Use of field flags in struct object

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

Show 14 quoted lines

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