Andre Przywara
2011-Jan-28 08:49 UTC
[Xen-devel] [PATCH] xl: fix incorrect display of illegal option character
Hi, according to the getopt(3) manpage (and to my testing) getopt returns ''?'' if an unknown option character is found and stores the insulting character in optopt. This patch fixes the broken output in such a situation: root@dosorca:/data/images# xl vcpu-list -j option `?'' not supported. Name ID VCPU CPU State Time(s) CPU Affinity Domain-0 0 0 0 -b- 193.1 any cpu turns into: root@dosorca:/data/images# xl vcpu-list -j option `j'' not supported. Name ID VCPU CPU State Time(s) CPU Affinity Domain-0 0 0 0 -b- 193.1 any cpu Please apply to 4.1.0-rc. By the way: some command parsers totally omit the faulting character and just output "option not supported." Would you still accept a patch which turns all of those occurrences into the upper, more verbose form? Especially as those conditions are not considered fatal and the command execution continues anyway, I found it rather confusing to read an unspecific error message without giving me a clue what I did wrong. Regards, Andre. Signed-off-by: Andre Przywara <andre.przywara@amd.com> -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jan-28 17:57 UTC
[Xen-devel] Re: [PATCH] xl: fix incorrect display of illegal option character
Andre Przywara writes ("[PATCH] xl: fix incorrect display of illegal option character"):> according to the getopt(3) manpage (and to my testing) getopt returns > ''?'' if an unknown option character is found and stores the insulting > character in optopt. > This patch fixes the broken output in such a situation:...> Please apply to 4.1.0-rc.Done, thanks.> By the way: some command parsers totally omit the faulting character and > just output "option not supported." > Would you still accept a patch which turns all of those occurrences into > the upper, more verbose form?Hrm, I''m not sure. I guess if the patch is very very obvious.> Especially as those conditions are not considered fatal and the command > execution continues anyway, I found it rather confusing to read an > unspecific error message without giving me a clue what I did wrong.TBH I think it is a bug when unknown options are not fatal. But it''s probably not sensible to change this now. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Andre Przywara
2011-Jan-28 22:53 UTC
[Xen-devel] [PATCH] xl: output illegal option character
On 01/28/2011 06:57 PM, Ian Jackson wrote:>> By the way: some command parsers totally omit the faulting character and >> just output "option not supported." >> Would you still accept a patch which turns all of those occurrences into >> the upper, more verbose form? > > Hrm, I''m not sure. I guess if the patch is very very obvious.What do you think about the attached one? Looks quite obvious to me.> >> Especially as those conditions are not considered fatal and the command >> execution continues anyway, I found it rather confusing to read an >> unspecific error message without giving me a clue what I did wrong. > > TBH I think it is a bug when unknown options are not fatal. But > it''s probably not sensible to change this now.Looking at the two patches I hope I can refactor the option parsing into common code after the release. This should be easier to fix then. Regards, Andre. ---------- Though illegal characters on xl command lines are catched, the user isn''t currently informed which one was not right. This patch fixes this by printing the faulting character. Signed-off-by: Andre Przywara <andre.przywara@amd.com> -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Feb-01 19:06 UTC
[Xen-devel] Re: [PATCH] xl: output illegal option character
Andre Przywara writes ("[PATCH] xl: output illegal option character"):> What do you think about the attached one? Looks quite obvious to me.Yes, applied, thanks.> Looking at the two patches I hope I can refactor the option parsing into > common code after the release. This should be easier to fix then.That would be lovely. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel