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

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);
 		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

On (Fri) 08 May 2015 [12:16:25], Dan Carpenter wrote:
> My static checker complains that this sprintf() can overflow but really
> it can't.  Just silence the warning by using snprintf().
Reviewed-by: Amit Shah <amit.shah at redhat.com>

Thanks!


		Amit

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
>

On Fri, May 08, 2015 at 11:30:09AM +0200, 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.
That's an user API change so it's probably a bad idea.

regards,
dan carpenter

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