[ovs-dev] [PATCH 2/2] netdev-dpdk: fix snprintf call

Aaron Conole aconole at redhat.com
Thu Jun 14 21:04:20 UTC 2018


Ben Pfaff <blp at ovn.org> writes:

> On Wed, Jun 13, 2018 at 03:43:04PM -0400, Aaron Conole 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);
>>                                                 ^
>> lib/netdev-dpdk.c:2865:9: note:  output between 13 and 17 bytes into a destination of size 16
>>         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).
>> 
>> Signed-off-by: Aaron Conole <aconole at redhat.com>
>> ---
>>  lib/netdev-dpdk.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 2e2f568b8..e75943bb2 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -2859,7 +2859,7 @@ 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];
>> +        char vhost_vring[18];
>>  
>>          rte_vhost_get_vhost_vring(vid, i, &vring);
>>          snprintf(vhost_vring, 16, "vring_%d_size", i);
>
> Thanks for the improvement.
>
> Instead of calculating at all, I think it would be less error-prone to
> avoid it, something like this:
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index fd496592ba56..040b17f8a34a 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2883,11 +2883,9 @@ 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[18];
> -
> -        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);
>
> What do you think?

That looks *much* better.  I completely missed that API.

> Thanks,
>
> Ben.


More information about the dev mailing list