Re: [Xen-devel] [PATCH v4 23/26] tools: Utility for dealing with featuresets

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Wed, Mar 23, 2016 at 04:36:26PM +0000, Andrew Cooper wrote:
> It is able to reports the current featuresets; both the static masks and
> dynamic featuresets from Xen, or to decode an arbitrary featureset into
> `/proc/cpuinfo` style strings.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>

.. snip..
> +int main(int argc, char **argv)
> +{
> +    enum { MODE_UNKNOWN, MODE_INFO, MODE_DETAIL, MODE_INTERPRET }
> +    mode = MODE_UNKNOWN;
> +
> +    nr_features = xc_get_cpu_featureset_size();
> +
> +    for ( ;; )
> +    {
> +        int option_index = 0, c;
> +        static struct option long_options[] =
> +        {
> +            { "help", no_argument, NULL, 'h' },
> +            { "info", no_argument, NULL, 'i' },
> +            { "detail", no_argument, NULL, 'd' },
> +            { "verbose", no_argument, NULL, 'v' },
> +            { NULL, 0, NULL, 0 },
> +        };
> +
> +        c = getopt_long(argc, argv, "hidv", long_options, &option_index);
> +
> +        if ( c == -1 )
> +            break;
> +
> +        switch ( c )
> +        {
> +        case 'h':
> + option_error:
> +            printf("Usage: %s [ info | detail | <featureset>* ]\n", argv[0]);
> +            return 0;
> +
> +        case 'i':
> +            mode = MODE_INFO;
> +            break;
> +
> +        case 'd':
> +        case 'v':
> +            mode = MODE_DETAIL;
> +            break;
> +
> +        default:
> +            printf("Bad option '%c'\n", c);
> +            goto option_error;

Oh my. An backward goto! How about moving this default right above 'case 'h'
and do a fallthrough?

Granted one could consider that even worst looking that this goto.

Either way:

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

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