[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 3/4] tools: add tools support for Intel CDP
On Fri, 2015-09-25 at 17:53 +0800, He Chen wrote: > > Quoting the relevant bits of code for clarity: > > libxl_psr_cbm_type type = LIBXL_PSR_CBM_TYPE_L3_CBM; > > ... > > case 'd': > > type = LIBXL_PSR_CBM_TYPE_L3_DATA; > > opt_data = 1; > > break; > > case 'c': > > type = LIBXL_PSR_CBM_TYPE_L3_CODE; > > opt_code = 1; > > break; > > } > > > > if (opt_data && opt_code) > > type = LIBXL_PSR_CBM_TYPE_L3_CBM; > > > > So the behaviour if -d and -c are given is exactly the same as if > > neither > > of them were given, i.e. type = LIBXL_PSR_CBM_TYPE_L3_CBM? Is that > > correct > > and intended? > > Yes. > > > If so then I think it would be clearer to only set opt_* during option > > parsing and then to figure out the correct LIBXL_PSR_CBM_TYPE_* > > explicitly > > afterwards, rather than have the code cycle through data->code->cbm. > > > > Or just outlaw passing both -d and -c together since it is confusing > > and > > equivalent to passing neither anyway. > > Yes, as you said, if user just passes one option -d (or -c), things would > be done during option parsing, there is no need to add the if(). > > But the key point is that I am not sure how to address outlaw passing > both > -d and -c together (is it allowed?). If it is permitted, the behaviour is > the same as passing neither indeed, and the if() is needed to avoid > latter > option overwritting former option. > > What's your suggestion? Sorry, I am a little confused. > Omit former opiton when both options are given and remove if()? > Or something else? I was trying to make one suggestion for restructuring the code and one design choice to make, let me see if I can clarify. I think the basic code structure should be: libxl_psr_cbm_type type; int opt_data = 0, opt_code = 1; [...] case 'd': opt_data = 1; break; case 'c': opt_code = 1; break; [...] [... now figure out correct type= based on opt_data + opt+code... ] Which separates the option parsing from the logic of what they mean. Then the choice I mentioned is whether passing -c and -d at the same time is valid or not. If you want passing both -c and -d at the same time to be invalid then the code would be something like: if (opt_data && opt_code) { log error and exit } else if (opt_data) { type = LIBXL_PSR_CBM_TYPE_L3_DATA; } else if (opt_code) { type = LIBXL_PSR_CBM_TYPE_L3_CODE; else { /* Neither */ type = LIBXL_PSR_CBM_TYPE_L3_CBM; } If you want passing both -c and -d to be valid and behave like passing neither then it would be something like: if (opt_data && opt_code) { type = LIBXL_PSR_CBM_TYPE_L3_CBM; } else if (opt_data) { type = LIBXL_PSR_CBM_TYPE_L3_DATA; } else if (opt_code) { type = LIBXL_PSR_CBM_TYPE_L3_CODE; else { /* Neither, same as both */ type = LIBXL_PSR_CBM_TYPE_L3_CBM; } Which one you use is up to you, depending on what you think the most sensible semantics are. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |