[ovs-dev,v2] netdev-dpdk: fix snprintf call
diff mbox series

Message ID 20180615132012.25301-1-aconole@redhat.com
State Accepted
Headers show
Series
  • [ovs-dev,v2] netdev-dpdk: fix snprintf call
Related show

Commit Message

Aaron Conole June 15, 2018, 1:20 p.m. UTC
lib/netdev-dpdk.c: In function :
lib/netdev-dpdk.c:2865:49: warning:  output may be truncated before the last format character [-Wformat-truncation=]
        snprintf(vhost_vring, 16, "vring_%d_size", i);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Since vring_num is 16 bits, the largest value ever would only be 17 bytes,
including the terminating nul.  Stretch it to 18 bytes (as a precaution
against a signed value, which again would never happen).

Suggested-by: Ben Pfaff <blp@ovn.org>
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 lib/netdev-dpdk.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Ilya Maximets June 15, 2018, 2:56 p.m. UTC | #1
> lib/netdev-dpdk.c: In function :
> lib/netdev-dpdk.c:2865:49: warning:  output may be truncated before the last format character [-Wformat-truncation=]
>         snprintf(vhost_vring, 16, "vring_%d_size", i);
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Since vring_num is 16 bits, the largest value ever would only be 17 bytes,
> including the terminating nul.  Stretch it to 18 bytes (as a precaution
> against a signed value, which again would never happen).

Looks like commit message is a bit outdated. Last sentence doesn't
make sense in v2.

Other than that:
Acked-by: Ilya Maximets <i.maximets@samsung.com>

> 
> Suggested-by: Ben Pfaff <blp at ovn.org>
> Signed-off-by: Aaron Conole <aconole at redhat.com>
> ---
>  lib/netdev-dpdk.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index a76e489a2..1bde9cfe7 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2883,11 +2883,10 @@ netdev_dpdk_vhost_user_get_status(const struct netdev *netdev,
>  
>      for (int i = 0; i < vring_num; i++) {
>          struct rte_vhost_vring vring;
> -        char vhost_vring[16];
>  
>          rte_vhost_get_vhost_vring(vid, i, &vring);
> -        snprintf(vhost_vring, 16, "vring_%d_size", i);
> -        smap_add_format(args, vhost_vring, "%d", vring.size);
> +        smap_add_nocopy(args, xasprintf("vring_%d_size", i),
> +                        xasprintf("%d", vring.size));
>      }
>  
>      ovs_mutex_unlock(&dev->mutex);
> -- 
> 2.14.3
Ben Pfaff June 15, 2018, 6:26 p.m. UTC | #2
On Fri, Jun 15, 2018 at 05:56:28PM +0300, Ilya Maximets wrote:
> > lib/netdev-dpdk.c: In function :
> > lib/netdev-dpdk.c:2865:49: warning:  output may be truncated before the last format character [-Wformat-truncation=]
> >         snprintf(vhost_vring, 16, "vring_%d_size", i);
> >         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > Since vring_num is 16 bits, the largest value ever would only be 17 bytes,
> > including the terminating nul.  Stretch it to 18 bytes (as a precaution
> > against a signed value, which again would never happen).
> 
> Looks like commit message is a bit outdated. Last sentence doesn't
> make sense in v2.
> 
> Other than that:
> Acked-by: Ilya Maximets <i.maximets@samsung.com>

Thanks, I applied this to master.
Stokes, Ian Nov. 9, 2018, 6:02 p.m. UTC | #3
> lib/netdev-dpdk.c: In function :
> lib/netdev-dpdk.c:2865:49: warning:  output may be truncated before the
> last format character [-Wformat-truncation=]
>         snprintf(vhost_vring, 16, "vring_%d_size", i);
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Since vring_num is 16 bits, the largest value ever would only be 17 bytes,
> including the terminating nul.  Stretch it to 18 bytes (as a precaution
> against a signed value, which again would never happen).
> 

This issue also occurs on branch 2.9, I'll backport to address this.

Thanks
Ian

> Suggested-by: Ben Pfaff <blp@ovn.org>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>  lib/netdev-dpdk.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> a76e489a2..1bde9cfe7 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2883,11 +2883,10 @@ netdev_dpdk_vhost_user_get_status(const struct
> netdev *netdev,
> 
>      for (int i = 0; i < vring_num; i++) {
>          struct rte_vhost_vring vring;
> -        char vhost_vring[16];
> 
>          rte_vhost_get_vhost_vring(vid, i, &vring);
> -        snprintf(vhost_vring, 16, "vring_%d_size", i);
> -        smap_add_format(args, vhost_vring, "%d", vring.size);
> +        smap_add_nocopy(args, xasprintf("vring_%d_size", i),
> +                        xasprintf("%d", vring.size));
>      }
> 
>      ovs_mutex_unlock(&dev->mutex);
> --
> 2.14.3
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Patch
diff mbox series

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index a76e489a2..1bde9cfe7 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2883,11 +2883,10 @@  netdev_dpdk_vhost_user_get_status(const struct netdev *netdev,
 
     for (int i = 0; i < vring_num; i++) {
         struct rte_vhost_vring vring;
-        char vhost_vring[16];
 
         rte_vhost_get_vhost_vring(vid, i, &vring);
-        snprintf(vhost_vring, 16, "vring_%d_size", i);
-        smap_add_format(args, vhost_vring, "%d", vring.size);
+        smap_add_nocopy(args, xasprintf("vring_%d_size", i),
+                        xasprintf("%d", vring.size));
     }
 
     ovs_mutex_unlock(&dev->mutex);