> -----Original Message----- > From: Peter Krempa [mailto:pkrempa@xxxxxxxxxx] > Sent: Wednesday, October 30, 2013 10:04 PM > To: Eric Blake; Chen Hanxiao; libvir-list@xxxxxxxxxx > Subject: Re: [PATCH]virsh: track alias option and improve error message > when option duplicate its alias > > On 10/30/13 14:59, Eric Blake wrote: > > On 10/30/2013 07:41 AM, Peter Krempa wrote: > > > >>> if ((*opts_seen & (1 << i)) && opt->type != VSH_OT_ARGV) > { > >>> - vshError(ctl, _("option --%s already seen"), name); > >>> - goto cleanup; > >>> + if ((*opts_seen & (1 << (i - 1)))) { > >>> + vshError(ctl, _("option '--%s' duplicates its alias > '--%s'"), > >>> + cmd->opts[i].name, > cmd->opts[i-1].name); > >> > >> This is not right. This code depends on the aliased option being right > >> before the target option in the array describing the options. If you > >> move the options in the array around you may get strange error messages. > > > > We already have a check baked into vshCmddefOptParse that guarantees > > that all alias options appear in the list earlier than what the alias > > expands to, so the rest of the code base is free to rely on that. > > > >> > >> The idea is good, but you need to make sure that the code doesn't depend > >> on ordering of the options in the array. > > > > I haven't looked closely at the patch, but we CAN depend on the order of > > options within the array. > > Either way, offset of "-1" to the current option can't be guaranteed in > case we have for example multiple aliases for the same main command or > just put some in between, when the aliases will be before the command > but not _RIGHT_ before. (I actually tried it.) > Thanks. v2 patch will solve this issue. > Peter > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list