| Message ID | 20180717133415.23781-7-jkbs@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Get rid of ctl_fatal() calls in ovn-nbctl (part 2) | expand |
On Tue, Jul 17, 2018 at 03:34:10PM +0200, Jakub Sitnicki wrote: > Let the caller handle the error. This prepares us for reporting errors > in daemon mode. > > Signed-off-by: Jakub Sitnicki <jkbs@redhat.com> I got a lot of "possibly uninitialized" warnings from GCC for this one: ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_qos_del’: ../ovn/utilities/ovn-nbctl.c:2068:17: error: ‘direction’ may be used uninitialized in this function [-Werror=maybe-uninitialized] if (strcmp(direction, ls->qos_rules[i]->direction)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_acl_add’: ../ovn/utilities/ovn-nbctl.c:1739:5: error: ‘direction’ may be used uninitialized in this function [-Werror=maybe-uninitialized] nbrec_acl_set_direction(acl, direction); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_acl_del’: ../ovn/utilities/ovn-nbctl.c:1829:17: error: ‘direction’ may be used uninitialized in this function [-Werror=maybe-uninitialized] if (strcmp(direction, acls[i]->direction)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_qos_add’: ../ovn/utilities/ovn-nbctl.c:1994:5: error: ‘direction’ may be used uninitialized in this function [-Werror=maybe-uninitialized] nbrec_qos_set_direction(qos, direction); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../ovn/utilities/ovn-nbctl.c: At top level: I think it's wrong but the following incremental solved it so I folded it in: diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index c18fa28256af..a4a533740cb9 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -1680,6 +1680,7 @@ parse_direction(const char *arg, const char **direction_p) } else if (arg[0] == 'f') { *direction_p = "from-lport"; } else { + *direction_p = NULL; return xasprintf("%s: direction must be \"to-lport\" or " "\"from-lport\"", arg); } Thanks, Ben.
On Mon, 23 Jul 2018 15:33:26 -0700 Ben Pfaff <blp@ovn.org> wrote: > On Tue, Jul 17, 2018 at 03:34:10PM +0200, Jakub Sitnicki wrote: > > Let the caller handle the error. This prepares us for reporting errors > > in daemon mode. > > > > Signed-off-by: Jakub Sitnicki <jkbs@redhat.com> > > I got a lot of "possibly uninitialized" warnings from GCC for this one: > > ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_qos_del’: > ../ovn/utilities/ovn-nbctl.c:2068:17: error: ‘direction’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > if (strcmp(direction, ls->qos_rules[i]->direction)) { > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_acl_add’: > ../ovn/utilities/ovn-nbctl.c:1739:5: error: ‘direction’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > nbrec_acl_set_direction(acl, direction); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_acl_del’: > ../ovn/utilities/ovn-nbctl.c:1829:17: error: ‘direction’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > if (strcmp(direction, acls[i]->direction)) { > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_qos_add’: > ../ovn/utilities/ovn-nbctl.c:1994:5: error: ‘direction’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > nbrec_qos_set_direction(qos, direction); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ../ovn/utilities/ovn-nbctl.c: At top level: > > I think it's wrong but the following incremental solved it so I folded > it in: > > diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c > index c18fa28256af..a4a533740cb9 100644 > --- a/ovn/utilities/ovn-nbctl.c > +++ b/ovn/utilities/ovn-nbctl.c > @@ -1680,6 +1680,7 @@ parse_direction(const char *arg, const char **direction_p) > } else if (arg[0] == 'f') { > *direction_p = "from-lport"; > } else { > + *direction_p = NULL; > return xasprintf("%s: direction must be \"to-lport\" or " > "\"from-lport\"", arg); > } > Thank you for fixing it up. It's an oversight on my side, I've only build tested with Clang. Wondering why 0-day bot wasn't complaining. Maybe it's just newer versions of GCC that detect this. Thanks, Jakub
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index 1cee059f2..c18fa2825 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -1671,17 +1671,19 @@ qos_cmp(const void *qos1_, const void *qos2_) } } -static const char * -parse_direction(const char *arg) +static char * OVS_WARN_UNUSED_RESULT +parse_direction(const char *arg, const char **direction_p) { /* Validate direction. Only require the first letter. */ if (arg[0] == 't') { - return "to-lport"; + *direction_p = "to-lport"; } else if (arg[0] == 'f') { - return "from-lport"; + *direction_p = "from-lport"; } else { - ctl_fatal("%s: direction must be \"to-lport\" or \"from-lport\"", arg); + return xasprintf("%s: direction must be \"to-lport\" or " + "\"from-lport\"", arg); } + return NULL; } static char * OVS_WARN_UNUSED_RESULT @@ -1710,7 +1712,12 @@ nbctl_acl_add(struct ctl_context *ctx) return; } - const char *direction = parse_direction(ctx->argv[2]); + const char *direction; + error = parse_direction(ctx->argv[2], &direction); + if (error) { + ctx->error = error; + return; + } int64_t priority; error = parse_priority(ctx->argv[3], &priority); if (error) { @@ -1804,7 +1811,11 @@ nbctl_acl_del(struct ctl_context *ctx) return; } - const char *direction = parse_direction(ctx->argv[2]); + const char *direction; + error = parse_direction(ctx->argv[2], &direction); + if (error) { + ctl_fatal("%s", error); + } size_t n_acls = pg ? pg->n_acls : ls->n_acls; struct nbrec_acl **acls = pg ? pg->acls : ls->acls; @@ -1917,13 +1928,18 @@ static void nbctl_qos_add(struct ctl_context *ctx) { const struct nbrec_logical_switch *ls; - const char *direction = parse_direction(ctx->argv[2]); + const char *direction; int64_t priority; int64_t dscp = -1; int64_t rate = 0; int64_t burst = 0; char *error; + error = parse_direction(ctx->argv[2], &direction); + if (error) { + ctx->error = error; + return; + } error = parse_priority(ctx->argv[3], &priority); if (error) { ctx->error = error; @@ -2034,7 +2050,12 @@ nbctl_qos_del(struct ctl_context *ctx) return; } - const char *direction = parse_direction(ctx->argv[2]); + const char *direction; + error = parse_direction(ctx->argv[2], &direction); + if (error) { + ctx->error = error; + return; + } /* If priority and match are not specified, delete all qos_rules with the * specified direction. */
Let the caller handle the error. This prepares us for reporting errors in daemon mode. Signed-off-by: Jakub Sitnicki <jkbs@redhat.com> --- ovn/utilities/ovn-nbctl.c | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-)