All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andre Przywara <andre.przywara@amd.com>
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: xen-devel <xen-devel@lists.xensource.com>
Subject: [PATCH] xl: output illegal option character
Date: Fri, 28 Jan 2011 23:53:03 +0100	[thread overview]
Message-ID: <4D4348CF.6070001@amd.com> (raw)
In-Reply-To: <19779.896.459909.425993@mariner.uk.xensource.com>

[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]

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


[-- Attachment #2: xl_output_illegal_character.patch --]
[-- Type: text/plain, Size: 7070 bytes --]

diff -r 52e928af3637 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Fri Jan 28 19:37:49 2011 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Fri Jan 28 23:40:38 2011 +0100
@@ -1762,7 +1762,7 @@
             help("mem-max");
             exit(0);
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -1809,7 +1809,7 @@
             help("mem-set");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -1875,7 +1875,7 @@
             help("cd-eject");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -1903,7 +1903,7 @@
             help("cd-insert");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -1944,7 +1944,7 @@
             num = atoi(optarg);
             break;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -1993,7 +1993,7 @@
             help("vncviewer");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -2032,7 +2032,7 @@
             help("pci-list-assignable-devices");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -2069,7 +2069,7 @@
             help("pci-list");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -2114,7 +2114,7 @@
             force = 1;
             break;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -2155,7 +2155,7 @@
             help("pci-attach");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -2816,7 +2816,7 @@
             help("restore");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -2865,7 +2865,7 @@
             debug = 1;
             break;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -2894,7 +2894,7 @@
             help("save");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -2938,7 +2938,7 @@
             debug = 1;
             break;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -2998,7 +2998,7 @@
             help("pause");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -3025,7 +3025,7 @@
             help("unpause");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -3051,7 +3051,7 @@
             help("destroy");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -3081,7 +3081,7 @@
             wait = 1;
             break;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -3107,7 +3107,7 @@
             help("reboot");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -3153,7 +3153,7 @@
             verbose = 1;
             break;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -3204,7 +3204,7 @@
             help("list-vm");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -3262,7 +3262,7 @@
             quiet = 1;
             break;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -3328,7 +3328,7 @@
             help("button-press");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -4103,7 +4103,7 @@
             help("debug-keys");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -4138,7 +4138,7 @@
             help("dmesg");
             return 0;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -5219,7 +5219,7 @@
             dryrun = 1;
             break;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }
@@ -5416,7 +5416,7 @@
             opt_cpus = 1;
             break;
         default:
-            fprintf(stderr, "option not supported\n");
+            fprintf(stderr, "option `%c' not supported.\n", optopt);
             break;
         }
     }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

  reply	other threads:[~2011-01-28 22:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-28  8:49 [PATCH] xl: fix incorrect display of illegal option character Andre Przywara
2011-01-28 17:57 ` Ian Jackson
2011-01-28 22:53   ` Andre Przywara [this message]
2011-02-01 19:06     ` [PATCH] xl: output " Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D4348CF.6070001@amd.com \
    --to=andre.przywara@amd.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.