diff mbox series

[ovs-dev,1/1] ovn-nbctl: Initialize arguments to avoid compilation warnings.

Message ID 1532527243-10796-1-git-send-email-ian.stokes@intel.com
State Accepted
Headers show
Series [ovs-dev,1/1] ovn-nbctl: Initialize arguments to avoid compilation warnings. | expand

Commit Message

Stokes, Ian July 25, 2018, 2 p.m. UTC
Output arguments for parse_priority() and dhcp_options_get() may not be
initialized when either function returns an error.

This causes compilation warnings for GCC 6.3.x regarding use of
uninitialized variable use and null-pointer-arithmetic.

Fix this by initializing priority_p* value to 0 for priority_parse()
when an error occurs during parsing.

For dhcp_options_get() set *dhcp_opts_p = dhcp_opts regardless as
dhcp_opts will be equal to NULL when an error occurs within the function
anyhow.

Cc: Jakub Sitnicki <jkbs@redhat.com>
Fixes: 3844c85de979 ("ovn-nbctl: Don't die in dhcp_options_get()."
Fixes: bc8223df3b01 ("ovn-nbctl: Don't die in parse_priority().")
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 ovn/utilities/ovn-nbctl.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Jakub Sitnicki July 26, 2018, 8:05 a.m. UTC | #1
On Wed, 25 Jul 2018 15:00:43 +0100
Ian Stokes <ian.stokes@intel.com> wrote:

> Output arguments for parse_priority() and dhcp_options_get() may not be
> initialized when either function returns an error.
> 
> This causes compilation warnings for GCC 6.3.x regarding use of
> uninitialized variable use and null-pointer-arithmetic.
> 
> Fix this by initializing priority_p* value to 0 for priority_parse()
> when an error occurs during parsing.
> 
> For dhcp_options_get() set *dhcp_opts_p = dhcp_opts regardless as
> dhcp_opts will be equal to NULL when an error occurs within the function
> anyhow.
> 
> Cc: Jakub Sitnicki <jkbs@redhat.com>
> Fixes: 3844c85de979 ("ovn-nbctl: Don't die in dhcp_options_get()."
> Fixes: bc8223df3b01 ("ovn-nbctl: Don't die in parse_priority().")
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> ---

Thank you for fixing it up.

Acked-by: Jakub Sitnicki <jkbs@redhat.com>

>  ovn/utilities/ovn-nbctl.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index 3c3e582..c3703a3 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -1870,6 +1870,9 @@ parse_priority(const char *arg, int64_t *priority_p)
>      int64_t priority;
>      if (!ovs_scan(arg, "%"SCNd64, &priority)
>          || priority < 0 || priority > 32767) {
> +        /* Priority_p could be uninitialized as no valid priority was
> +         * input, initialize it to a valid value of 0 before returning */
> +        *priority_p = 0;
>          return xasprintf("%s: priority must in range 0...32767", arg);
>      }
>      *priority_p = priority;
> @@ -2899,10 +2902,11 @@ dhcp_options_get(struct ctl_context *ctx, const char *id, bool must_exist,
>          dhcp_opts = nbrec_dhcp_options_get_for_uuid(ctx->idl, &dhcp_opts_uuid);
>      }
>  
> +    *dhcp_opts_p = dhcp_opts;
>      if (!dhcp_opts && must_exist) {
>          return xasprintf("%s: dhcp options UUID not found", id);
>      }
> -    *dhcp_opts_p = dhcp_opts;
> +
>      return NULL;
>  }
>
Ben Pfaff July 31, 2018, 7:53 p.m. UTC | #2
On Thu, Jul 26, 2018 at 10:05:01AM +0200, Jakub Sitnicki wrote:
> On Wed, 25 Jul 2018 15:00:43 +0100
> Ian Stokes <ian.stokes@intel.com> wrote:
> 
> > Output arguments for parse_priority() and dhcp_options_get() may not be
> > initialized when either function returns an error.
> > 
> > This causes compilation warnings for GCC 6.3.x regarding use of
> > uninitialized variable use and null-pointer-arithmetic.
> > 
> > Fix this by initializing priority_p* value to 0 for priority_parse()
> > when an error occurs during parsing.
> > 
> > For dhcp_options_get() set *dhcp_opts_p = dhcp_opts regardless as
> > dhcp_opts will be equal to NULL when an error occurs within the function
> > anyhow.
> > 
> > Cc: Jakub Sitnicki <jkbs@redhat.com>
> > Fixes: 3844c85de979 ("ovn-nbctl: Don't die in dhcp_options_get()."
> > Fixes: bc8223df3b01 ("ovn-nbctl: Don't die in parse_priority().")
> > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > ---
> 
> Thank you for fixing it up.
> 
> Acked-by: Jakub Sitnicki <jkbs@redhat.com>

Thanks, applied to master and branch-2.10.
diff mbox series

Patch

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 3c3e582..c3703a3 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1870,6 +1870,9 @@  parse_priority(const char *arg, int64_t *priority_p)
     int64_t priority;
     if (!ovs_scan(arg, "%"SCNd64, &priority)
         || priority < 0 || priority > 32767) {
+        /* Priority_p could be uninitialized as no valid priority was
+         * input, initialize it to a valid value of 0 before returning */
+        *priority_p = 0;
         return xasprintf("%s: priority must in range 0...32767", arg);
     }
     *priority_p = priority;
@@ -2899,10 +2902,11 @@  dhcp_options_get(struct ctl_context *ctx, const char *id, bool must_exist,
         dhcp_opts = nbrec_dhcp_options_get_for_uuid(ctx->idl, &dhcp_opts_uuid);
     }
 
+    *dhcp_opts_p = dhcp_opts;
     if (!dhcp_opts && must_exist) {
         return xasprintf("%s: dhcp options UUID not found", id);
     }
-    *dhcp_opts_p = dhcp_opts;
+
     return NULL;
 }