[ovs-dev] [PATCH 1/3] utilities: Fix and unify parsing of timeout option.
Ilya Maximets
i.maximets at samsung.com
Tue Aug 14 07:53:15 UTC 2018
Parsing of the '--timeout' option implemented differently
for every single control utility and, which is more
important, highly inaccurate. In most cases unsigned result
of 'strtoul' stored in signed variable. Parsing failures are
not tracked. 'ovs-appctl' even uses just 'atoi' without any
checking of the argument or result.
This patch unifies the parsing by using 'str_to_uint'.
Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
---
ovn/utilities/ovn-nbctl.c | 8 +++-----
ovn/utilities/ovn-sbctl.c | 5 ++---
ovsdb/ovsdb-client.c | 8 +++-----
tests/test-ovsdb.c | 8 +++-----
utilities/ovs-appctl.c | 7 ++++++-
utilities/ovs-dpctl.c | 8 +++-----
utilities/ovs-ofctl.c | 8 +++-----
utilities/ovs-vsctl.c | 8 +++-----
vtep/vtep-ctl.c | 8 +++-----
9 files changed, 29 insertions(+), 39 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 35a3af3..972ba12 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -68,7 +68,7 @@ static enum nbctl_wait_type wait_type = NBCTL_WAIT_NONE;
static bool force_wait = false;
/* --timeout: Time to wait for a connection to 'db'. */
-static int timeout;
+static unsigned int timeout;
/* Format for table output. */
static struct table_style table_style = TABLE_STYLE_DEFAULT;
@@ -345,8 +345,7 @@ handle_main_loop_option(int opt, const char *arg, bool *handled)
break;
case 't':
- timeout = strtoul(arg, NULL, 10);
- if (timeout < 0) {
+ if (!str_to_uint(arg, 10, &timeout) || !timeout) {
return xasprintf("value %s on -t or --timeout is invalid", arg);
}
break;
@@ -5358,8 +5357,7 @@ nbctl_client(const char *socket_name,
exit(EXIT_SUCCESS);
case 't':
- timeout = strtoul(po->arg, NULL, 10);
- if (timeout < 0) {
+ if (!str_to_uint(po->arg, 10, &timeout) || !timeout) {
ctl_fatal("value %s on -t or --timeout is invalid", po->arg);
}
break;
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index 7022347..6ec4c80 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -69,7 +69,7 @@ static bool oneline;
static bool dry_run;
/* --timeout: Time to wait for a connection to 'db'. */
-static int timeout;
+static unsigned int timeout;
/* Format for table output. */
static struct table_style table_style = TABLE_STYLE_DEFAULT;
@@ -268,8 +268,7 @@ parse_options(int argc, char *argv[], struct shash *local_options)
exit(EXIT_SUCCESS);
case 't':
- timeout = strtoul(optarg, NULL, 10);
- if (timeout < 0) {
+ if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
ctl_fatal("value %s on -t or --timeout is invalid", optarg);
}
break;
diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index cdb021f..7f02188 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -332,7 +332,7 @@ parse_options(int argc, char *argv[])
table_style.format = TF_TABLE;
for (;;) {
- unsigned long int timeout;
+ unsigned int timeout = 0;
int c;
c = getopt_long(argc, argv, short_options, long_options, NULL);
@@ -366,10 +366,8 @@ parse_options(int argc, char *argv[])
break;
case 't':
- timeout = strtoul(optarg, NULL, 10);
- if (timeout <= 0) {
- ovs_fatal(0, "value %s on -t or --timeout is not at least 1",
- optarg);
+ if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
+ ovs_fatal(0, "value %s on -t or --timeout is invalid", optarg);
} else {
time_alarm(timeout);
}
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 7932204..4b216fe 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -96,7 +96,7 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt)
char *short_options = ovs_cmdl_long_options_to_short_options(long_options);
for (;;) {
- unsigned long int timeout;
+ unsigned int timeout = 0;
int c;
c = getopt_long(argc, argv, short_options, long_options, NULL);
@@ -106,10 +106,8 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt)
switch (c) {
case 't':
- timeout = strtoul(optarg, NULL, 10);
- if (timeout <= 0) {
- ovs_fatal(0, "value %s on -t or --timeout is not at least 1",
- optarg);
+ if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
+ ovs_fatal(0, "value %s on -t or --timeout is invalid", optarg);
} else {
time_alarm(timeout);
}
diff --git a/utilities/ovs-appctl.c b/utilities/ovs-appctl.c
index 8f87cc4..d6408ea 100644
--- a/utilities/ovs-appctl.c
+++ b/utilities/ovs-appctl.c
@@ -133,6 +133,7 @@ parse_command_line(int argc, char *argv[])
e_options = 0;
for (;;) {
int option;
+ unsigned int timeout = 0;
option = getopt_long(argc, argv, short_options, long_options, NULL);
if (option == -1) {
@@ -165,7 +166,11 @@ parse_command_line(int argc, char *argv[])
exit(EXIT_SUCCESS);
case 'T':
- time_alarm(atoi(optarg));
+ if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
+ ovs_fatal(0, "value %s on -T or --timeout is invalid", optarg);
+ } else {
+ time_alarm(timeout);
+ }
break;
case 'V':
diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index c4db97b..441f804 100644
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -101,7 +101,7 @@ parse_options(int argc, char *argv[])
bool set_names = false;
for (;;) {
- unsigned long int timeout;
+ unsigned int timeout = 0;
int c;
c = getopt_long(argc, argv, short_options, long_options, NULL);
@@ -141,10 +141,8 @@ parse_options(int argc, char *argv[])
break;
case 't':
- timeout = strtoul(optarg, NULL, 10);
- if (timeout <= 0) {
- ovs_fatal(0, "value %s on -t or --timeout is not at least 1",
- optarg);
+ if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
+ ovs_fatal(0, "value %s on -t or --timeout is invalid", optarg);
} else {
time_alarm(timeout);
}
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index c3b49db..708e6b2 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -272,7 +272,7 @@ parse_options(int argc, char *argv[])
set_allowed_ofp_versions("OpenFlow10");
for (;;) {
- unsigned long int timeout;
+ unsigned int timeout = 0;
int c;
c = getopt_long(argc, argv, short_options, long_options, NULL);
@@ -282,10 +282,8 @@ parse_options(int argc, char *argv[])
switch (c) {
case 't':
- timeout = strtoul(optarg, NULL, 10);
- if (timeout <= 0) {
- ovs_fatal(0, "value %s on -t or --timeout is not at least 1",
- optarg);
+ if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
+ ovs_fatal(0, "value %s on -t or --timeout is invalid", optarg);
} else {
time_alarm(timeout);
}
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index d14aa6c..523775c 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -69,7 +69,7 @@ static bool dry_run;
static bool wait_for_reload = true;
/* --timeout: Time to wait for a connection to 'db'. */
-static int timeout;
+static unsigned int timeout;
/* --retry: If true, ovs-vsctl will retry connecting to the database forever.
* If false and --db says to use an active connection method (e.g. "unix:",
@@ -311,10 +311,8 @@ parse_options(int argc, char *argv[], struct shash *local_options)
exit(EXIT_SUCCESS);
case 't':
- timeout = strtoul(optarg, NULL, 10);
- if (timeout < 0) {
- ctl_fatal("value %s on -t or --timeout is invalid",
- optarg);
+ if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
+ ctl_fatal("value %s on -t or --timeout is invalid", optarg);
}
break;
diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
index 35ab435..fb3c81e 100644
--- a/vtep/vtep-ctl.c
+++ b/vtep/vtep-ctl.c
@@ -65,7 +65,7 @@ static bool oneline;
static bool dry_run;
/* --timeout: Time to wait for a connection to 'db'. */
-static int timeout;
+static unsigned int timeout;
/* Format for table output. */
static struct table_style table_style = TABLE_STYLE_DEFAULT;
@@ -252,10 +252,8 @@ parse_options(int argc, char *argv[], struct shash *local_options)
exit(EXIT_SUCCESS);
case 't':
- timeout = strtoul(optarg, NULL, 10);
- if (timeout < 0) {
- ctl_fatal("value %s on -t or --timeout is invalid",
- optarg);
+ if (!str_to_uint(optarg, 10, &timeout) || !timeout) {
+ ctl_fatal("value %s on -t or --timeout is invalid", optarg);
}
break;
--
2.7.4
More information about the dev
mailing list