Re: [PATCH v5 2/3] advice: revamp advise API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> Heba Waly <heba.waly@xxxxxxxxx> writes:
> 
> > I'm not against this approach as well, but as I mentioned above, we
> > need a list of keys to be returned by list_config_advices(), that's
> > why defining the constant strings will not be sufficient in our case.
> 
> Sorry, but I do not get it.  
> 
> Either you use enum or a bunch of variables of type const char [],
> "list all of them" would need an array whose elements are all of
> them, so
> 
>         const char ADVICE_FOO[] = "advice.foo";
>         const char ADVICE_BAR[] = "advice.bar";
>         ...
> 
>         static const char *all_advice_type[] = {
>                 ADVICE_FOO, ADVICE_BAR, ...
>         };
> 
> 	void for_each_advice_type(int (*fn)(const char *name))
> 	{
> 		int i;
> 		for (i = 0; i < ARRAY_SIZE(all_advice_type); i++)
> 			fn(all_advice_type[i]);
> 	}
> 
> would be sufficient, and I do not think it takes any more effort to
> create and manage than using an array indexed with the enum, no?

With the enum:

(.h)
enum advice_type {
	ADVICE_FOO,
	ADVICE_BAR
};

(.c)
static const char *advice_config_keys[] = {
	[ADVICE_FOO] = "advice.foo",
	[ADVICE_BAR] = "advice.bar"
};
/* No need for all_advice_type because we can loop over advice_config_keys */

With the bunch of variables of type const char []:

(.h)
extern const char ADVICE_FOO[];
extern const char ADVICE_BAR[];

(.c)
const char ADVICE_FOO[] = "advice.foo";
const char ADVICE_BAR[] = "advice.bar";
static const char *all_advice_type[] = {
	ADVICE_FOO,
	ADVICE_BAR
};

Junio, is this what you meant? It seems to me that there is an extra array to
be managed in the latter case. Admittedly, this is a tradeoff against needing
to convert the enum to a string when checking config, as you describe [1].

[1] https://lore.kernel.org/git/xmqq7e09hydx.fsf@xxxxxxxxxxxxxxxxxxxxxxxxx/



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux