[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 1/2] tools/xl: Sort create command options
On Fri, Apr 29, 2022 at 10:39:27AM +0100, Anthony PERARD wrote: > On Tue, Apr 19, 2022 at 06:56:03PM -0700, Elliott Mitchell wrote: > > Hopefully simplify future changes by sorting options lists for > > `xl create`. > > I'm not sure that sorting options make changes easier, as it would mean > one has to make sure the new option is sorted as well ;-). But sorting > the options in the help message is probably useful; I've looked at a few > linux utilities and they tend to have them sorted so doing this for xl > create sound fine. This ends up revolving around the question, is the work involved in keeping things sorted more or less than the annoyance caused by having them unsorted? I tend towards keep them sorted since I find trying to identify available option letters when they're in random order is rather troublesome. > > Signed-off-by: Elliott Mitchell <ehem+xen@xxxxxxx> > > --- > > diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c > > index 435155a033..2ec4140258 100644 > > --- a/tools/xl/xl_vmcontrol.c > > +++ b/tools/xl/xl_vmcontrol.c > > @@ -1169,13 +1169,13 @@ int main_create(int argc, char **argv) > > int paused = 0, debug = 0, daemonize = 1, console_autoconnect = 0, > > quiet = 0, monitor = 1, vnc = 0, vncautopass = 0, ignore_masks = 0; > > int opt, rc; > > - static struct option opts[] = { > > + static const struct option opts[] = { > > Could you add a note in the commit message that "opts" is now > const? Okay. > > + {"defconfig", 1, 0, 'f'}, > > {"dryrun", 0, 0, 'n'}, > > + {"ignore-global-affinity-masks", 0, 0, 'i'}, > > {"quiet", 0, 0, 'q'}, > > - {"defconfig", 1, 0, 'f'}, > > {"vncviewer", 0, 0, 'V'}, > > {"vncviewer-autopass", 0, 0, 'A'}, > > - {"ignore-global-affinity-masks", 0, 0, 'i'}, > > COMMON_LONG_OPTS > > }; > > > > @@ -1186,12 +1186,15 @@ int main_create(int argc, char **argv) > > argc--; argv++; > > } > > > > - SWITCH_FOREACH_OPT(opt, "Fnqf:pcdeVAi", opts, "create", 0) { > > - case 'f': > > - filename = optarg; > > + SWITCH_FOREACH_OPT(opt, "Ffnq:AVcdeip", opts, "create", 0) { > > The list of short options aren't really sorted here. Also -q doesn't > take an argument, but -f should keep requiring one. Needed to reread the documentation on getopt() behavior. I remembered it being group before the colon didn't take options, ones after colon did take options. Instead it is colon for every option with argument. Other issue is dictionary sort versus ASCII sort. ie "AaBbCcDdEeFf..." versus "A-Za-z". -- (\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/) \BS ( | ehem+sigmsg@xxxxxxx PGP 87145445 | ) / \_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |