Re: [Xen-devel] [PATCH 2 of 2] xl: Introduce helper macro for option parsing

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("[PATCH 2 of 2] xl: Introduce helper macro for option 
parsing"):
> - s/FOREACH_OPT/SWITCH_FOREACH_OPT/
> - Document the macro

Thanks.

> +/*
> + * Wraps def_getopt into a convenient loop+switch to process all arguments.
> + *
> + * _opt:        an int variable, holds the current option during processing.
> + * _opts:       short options, as per getopt_long(3)'s optstring argument.
> + * _lopts:      long options, as per getopt_long(3)'s longopts argument. May
> + *              be null.
> + * _help:       name of this command, for usage string.
> + * _req:        number of non-option command line parameters which are 
> required.

Can we have a pseudo-prototype for this ?  Eg

    *   SWITCH_FOREACH_OPT(int &opt, const char *opts,
    *                      const struct option *longopts,
    *                      const char *commandname,
    *                      int num_opts_req) { ...
    *    case ...

Also, there is no need to prefix macro formal arguments with _.  Why
do you do that ?  We don't do it elsewhere in libxl...

> + * Callers should treat SWITCH_FOREACH_OPT as they would a switch
> + * statement over the value of _opt. Each option given in _opts (or
> + * _lopts) should be handled by a case statement as if it were inside
> + * a switch statement.
> + *
> + * In addition to the options provided in _opts callers must handle
> + * two additional pseudo options:
> + *  0 -- generated if the user passes a -h option. help will be printed,
> + *       caller should return 0.
> + *  2 -- generated if the user does not provided _req non-option arguments,
> + *       caller should return 2.

I don't think you can mean "caller should return".  Your description
of the macro doesn't specify anything in particular about the calling
function so it can't possibly intend for you to return particular
values from it.

Did you mean "cause the program to exit" ?  And if so why not have the
macro (or the function) do that ?

I haven't gone through your call sites to review them...

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel