Re: [PATCH 09/20] parse-options: add support for parsing subcommands

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

 



On Mon, Jul 25, 2022 at 09:41:52PM +0200, Ævar Arnfjörð Bjarmason wrote:
> >> > @@ -206,6 +217,11 @@ struct option {
> >> >  #define OPT_ALIAS(s, l, source_long_name) \
> >> >  	{ OPTION_ALIAS, (s), (l), (source_long_name) }
> >> >  
> >> > +#define OPT_SUBCOMMAND(l, v, fn)      { OPTION_SUBCOMMAND, 0, (l), (v), NULL, \
> >> > +					NULL, 0, NULL, 0, NULL, 0, (fn) }
> >> > +#define OPT_SUBCOMMAND_F(l, v, fn, f) { OPTION_SUBCOMMAND, 0, (l), (v), NULL, \
> >> > +					NULL, (f), NULL, 0, NULL, 0, (fn) }
> >> 
> >> Nit, I know you're carrying forward existing patterns, but since that
> >> all pre-dated designated init perhaps we could just (untested):
> >> 	
> >> 	#define OPT_SUBCOMMAND_F(l, v, fn, f) { \
> >> 		.type = OPTION_SUBCOMMAND, \
> >> 		.long_name = (l), \
> >> 		.value = (v), \
> >> 		.ll_callback = (fn), \
> >> 	}
> >> 	#define OPT_SUBCOMMAND(l, v, fn) OPT_SUBCOMMAND_F((l), (v), (fn), 0)
> >> 
> >> Which IMO is much nicer. I have some patches somewhere to convert these
> >> to saner patterns (I think not designated init, but the X() can be
> >> defined in terms of X_F() like that, but since this is new we can use
> >> designated init all the way...
> >
> > Oh, I love this idea!  But are we there yet?  I remember the weather
> > balloon about designated initializers, but I'm not sure whether we've
> > already made the decision to allow them.
> 
> Yes, we've got a thoroughly hard dependency on that part of C99 for a
> while now, and it's OK to add new ones (especially in cases like these,
> where it makes thigs easier to read).

Good.  I updated this hunk to use designated initializers as you
suggested, because all those unused 0/NULL fields in there are just...
ugly.

> > If we do, then I'm inclined
> > to volunteer to clean up all those OPT_* macros in 'parse-options.h'
> > with designated initializers,

But I'll leave this for later, because it's awfully easy to make a
mistake and assign a macro parameter to the wrong field, and I find
the resulting diff very hard to review.




[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