[ovs-dev] ovn-nbctl: Detect unrecognized short options in server mode.

Message ID 20180725152654.18068-1-jkbs@redhat.com
State Accepted
Headers show
Series
  • [ovs-dev] ovn-nbctl: Detect unrecognized short options in server mode.
Related show

Commit Message

Jakub Sitnicki July 25, 2018, 3:26 p.m.
Because getopt() will set optopt for both known and unknown options,
we need to differentiate between them ourselves by checking if we
know the option. Do that by looking up its value.

Also, because we are using GNU extensions to getopt(), we need to be
resetting getopt() state by setting optind to 0 instead of 1 as
pointed out in NOTES in getopt(3) man-page. Not doing so results in
invalid reads and optopt being set to a garbarge value.

Fixes: 3ec06ea9c668 ("ovn-nbctl: Initial support for daemon mode.")
Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
---
 ovn/utilities/ovn-nbctl.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Comments

Ben Pfaff July 31, 2018, 7:35 p.m. | #1
On Wed, Jul 25, 2018 at 05:26:54PM +0200, Jakub Sitnicki wrote:
> Because getopt() will set optopt for both known and unknown options,
> we need to differentiate between them ourselves by checking if we
> know the option. Do that by looking up its value.
> 
> Also, because we are using GNU extensions to getopt(), we need to be
> resetting getopt() state by setting optind to 0 instead of 1 as
> pointed out in NOTES in getopt(3) man-page. Not doing so results in
> invalid reads and optopt being set to a garbarge value.
> 
> Fixes: 3ec06ea9c668 ("ovn-nbctl: Initial support for daemon mode.")
> Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>

Thanks, applied to master and branch-2.10.

Patch

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 8b888294e..a1717d153 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -4844,6 +4844,19 @@  nbctl_cmd_init(void)
     ctl_register_commands(nbctl_commands);
 }
 
+static const struct option *
+find_option_by_value(const struct option *options, int value)
+{
+    const struct option *o;
+
+    for (o = options; o->name; o++) {
+        if (o->val == value) {
+            return o;
+        }
+    }
+    return NULL;
+}
+
 static char * OVS_WARN_UNUSED_RESULT
 server_parse_options(int argc, char *argv[], struct shash *local_options,
                      int *n_options_p)
@@ -4863,7 +4876,7 @@  server_parse_options(int argc, char *argv[], struct shash *local_options,
     short_options = build_short_options(global_long_options, false);
     options = append_command_options(global_long_options, OPT_LOCAL);
 
-    optind = 1;
+    optind = 0;
     opterr = 0;
     for (;;) {
         int idx;
@@ -4899,9 +4912,11 @@  server_parse_options(int argc, char *argv[], struct shash *local_options,
         TABLE_OPTION_HANDLERS(&table_style)
 
         case '?':
-            if (optopt) {
+            if (find_option_by_value(options, optopt)) {
                 error = xasprintf("option '%s' doesn't allow an argument",
                                   argv[optind-1]);
+            } else if (optopt) {
+                error = xasprintf("unrecognized option '%c'", optopt);
             } else {
                 error = xasprintf("unrecognized option '%s'", argv[optind-1]);
             }