[sheepdog] [PATCH 3/6] introduce sd_getopt as an alternative to getopt_long

MORITA Kazutaka morita.kazutaka at gmail.com
Mon Nov 5 16:29:55 CET 2012


At Mon, 05 Nov 2012 22:39:00 +0800,
Liu Yuan wrote:
> 
> On 11/05/2012 06:52 PM, morita.kazutaka at gmail.com wrote:
> > +#define SD_OPT_NONE     0x00
> > +#define SD_OPT_STRING   0x01
> > +#define SD_OPT_NUMBER   0x02
> > +#define SD_OPT_BOOL     0x04
> > +#define SD_OPT_SIZE     0x08
> 
> Is this flags for check necessary? We already set 'type' of parameter at
> the very beginning (at sd_opt_param struct), so we can do type check at
> the parse phase and throw error message when meeting invalid parameters.
> So when we call sd_opt_param_get(), we should get what we specify at
> sd_opt_param struct and no need to check type after calling this function.

I tried the similar type check first, but I found that --snapshot
option of 'collie vdi xxx' allows both number and string; if optarg is
number, vdi_parser regards it as a snapshot id, and otherwise, the
function regards it as a snapshot tag.  So vdi_parser needs to know
how parse_sd_opt_value() parsed the arugment, and that's the reason I
gave up doing the type check in option.c.

Another idea I came up with was setting expecting types by or'ing
SD_OPT_XXX values, and receiving the parsed types as a result.
parse_sd_opt_value returns error if it fails in parsing the argument
as none of the expected types.  How do you think about this?

> 
> Maybe we can even do range check for 'size' at parse time by built-in
> 'min' 'max' information at sd_opt_param struct.

I also tried this, but gave up because this makes sd_opt_param
complicated.  But if you prefer this, I don't mind implementing it.

> 
> I'd suggest to name sd_opt_param_get() as sd_opt_get_value(prefix, key)
> to reflect the K:V pairs. This prefix can be NULL to save extra call of
> strcmp() inside for options like '-j {dir, size, skip}'

Okay.

Thanks,

Kazutaka



More information about the sheepdog mailing list