Linux Virtualization - May 2015 - [patch v2] virtio_console: silence a static checker warning

Am 08.05.2015 11:16, schrieb Dan Carpenter:
> My static checker complains that this sprintf() can overflow but really
> it can't.  Just silence the warning by using snprintf().
> 
> Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>
> ---
> v2: the overflow is not possible so just leave the buffer size alone and
>     silence the warning with snprintf().
> 
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 50754d20..8283989 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -1492,8 +1492,8 @@ static int add_port(struct ports_device *portdev, u32
id)
>  		 * Finally, create the debugfs file that we can use to
>  		 * inspect a port's state at any time
>  		 */
> -		sprintf(debugfs_name, "vport%up%u",
> -			port->portdev->vdev->index, id);
> +		snprintf(debugfs_name, sizeof(debugfs_name), "vport%up%u",
> +			 port->portdev->vdev->index, id);

would it help to use %03u (or so) to make it more obvious ?

Note: i prefer a leading 0 in my programms to make it more easy
to work with grep and friends. you may thing otherwise.


re,
 wh
>  		port->debugfs_file = debugfs_create_file(debugfs_name, 0444,
>  							 pdrvdata.debugfs_dir,
>  							 port,
> --
> To unsubscribe from this list: send the line "unsubscribe
kernel-janitors" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe
kernel-janitors" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Am 08.05.2015 11:56, schrieb Amit Shah:
> On (Fri) 08 May 2015 [11:30:09], walter harms wrote:
>>
>>
>> Am 08.05.2015 11:16, schrieb Dan Carpenter:
>>> My static checker complains that this sprintf() can overflow but
really
>>> it can't.  Just silence the warning by using snprintf().
>>>
>>> Signed-off-by: Dan Carpenter <dan.carpenter at oracle.com>
>>> ---
>>> v2: the overflow is not possible so just leave the buffer size
alone and
>>>     silence the warning with snprintf().
>>>
>>> diff --git a/drivers/char/virtio_console.c
b/drivers/char/virtio_console.c
>>> index 50754d20..8283989 100644
>>> --- a/drivers/char/virtio_console.c
>>> +++ b/drivers/char/virtio_console.c
>>> @@ -1492,8 +1492,8 @@ static int add_port(struct ports_device
*portdev, u32 id)
>>>  		 * Finally, create the debugfs file that we can use to
>>>  		 * inspect a port's state at any time
>>>  		 */
>>> -		sprintf(debugfs_name, "vport%up%u",
>>> -			port->portdev->vdev->index, id);
>>> +		snprintf(debugfs_name, sizeof(debugfs_name),
"vport%up%u",
>>> +			 port->portdev->vdev->index, id);
>>
>>
>> would it help to use %03u (or so) to make it more obvious ?
>>
>> Note: i prefer a leading 0 in my programms to make it more easy
>> to work with grep and friends. you may thing otherwise.
> 
> Well we've been exposing names like /dev/vport0p0, /dev/vport2p15,
> etc., and there might be scripts relying on such names, so that's one
> argument against it.
> 
> However we do have pretty names that map to these ports via udev
> rules, but not sure if we should change the name just to prepend 0s.
> 
> 		Amit
> 
The basic idea was to limit the space, having leading zeros is my idea because
i found this more convenient in the past. Using something like "%3u"
will give
static checkers a chance to detect the required max. space.

re,
 wh