Subject: Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag <snip>

From: SZEDER Gábor <[email-masked]>
Date: Mon, 7 Oct 2019 19:29:51 +0200

On Sat, Sep 28, 2019 at 12:40:52PM +0900, Junio C Hamano wrote:
> SZEDER Gábor <[email-masked]> writes:
> 
> > On Fri, Sep 20, 2019 at 09:53:48AM -0700, William Baker via GitGitGadget wrote:
> >> diff --git a/midx.h b/midx.h
> >> index f0ae656b5d..e6fa356b5c 100644
> >> --- a/midx.h
> >> +++ b/midx.h
> >> @@ -37,6 +37,8 @@ struct multi_pack_index {
> >>  	char object_dir[FLEX_ARRAY];
> >>  };
> >>  
> >> +#define MIDX_PROGRESS     (1 << 0)
> >
> > Please consider using an enum.
> 
> If they are used by assiging one of their values, definitely a good
> idea to use an enum.  Are debuggers clever enough that they can
> tell, when they see something like this:
> 
> 	enum gress {
> 		PROGRESS = 1,
> 		REGRESS = 2,
> 	};
> 
> 	void func(enum gress v);
> 
> 	...
> 
>         void caller(void)
> 	{
> 		func(PROGRESS | REGRESS);
> 		func(PROGRESS + REGRESS);
> 		func(PROGRESS * 3);
> 	}
> 
> how caller came about to give 3?

No, they tend to show (PROGRESS | REGRESS), at least both gdb and lldb
do.  If the enum has only constants with power-of-two values, then that
is the right way to write it, and the other two are asking for trouble
(e.g. after someone changes the value of REGRESS from 2 to 8,
'PROGRESS * 3' will mean something different).