[PATCH] char: xillybus: use strscpy() in init_chrdev

[PATCH] char: xillybus: use strscpy() in init_chrdev

[Thorsten Blum]

The format specifier is unnecessary and can be removed. Replace
snprintf("%s") with the faster and more direct strscpy().

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/char/xillybus/xillybus_class.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/char/xillybus/xillybus_class.c b/drivers/char/xillybus/xillybus_class.c
index c92a628e389e..bbdfa3e87514 100644
--- a/drivers/char/xillybus/xillybus_class.c
+++ b/drivers/char/xillybus/xillybus_class.c
@@ -11,6 +11,7 @@
 #include <linux/fs.h>
 #include <linux/cdev.h>
 #include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
 
@@ -65,7 +66,7 @@ int xillybus_init_chrdev(struct device *dev,
 	mutex_lock(&unit_mutex);
 
 	if (!enumerate)
-		snprintf(unit->name, UNITNAMELEN, "%s", prefix);
+		strscpy(unit->name, prefix);
 
 	for (i = 0; enumerate; i++) {
 		snprintf(unit->name, UNITNAMELEN, "%s_%02d",

Re: [PATCH] char: xillybus: use strscpy() in init_chrdev

[Eli Billauer]

Hello,

On 27/04/2026 19:37, Thorsten Blum wrote:

>   	if (!enumerate)
> -		snprintf(unit->name, UNITNAMELEN, "%s", prefix);
> +		strscpy(unit->name, prefix);
>   
>   	for (i = 0; enumerate; i++) {
>   		snprintf(unit->name, UNITNAMELEN, "%s_%02d",

snprintf() is used deliberately for code clarity: It makes a simple 
visual contrast with the use of snprintf() three rows below. As this 
call takes place only once for each physical hardware device handled by 
the driver, the advantage in optimizing this function call is negligible.

And even though it's formally OK to use strscpy() with two arguments, 
and let the compilation machinery find out the length of the char array, 
I have to admit that it gives me the chills. Buffer overflow and that.

So overall, I can't say I'm very fond of this patch. Thanks for the 
effort nevertheless.

Regards,
    Eli

Re: [PATCH] char: xillybus: use strscpy() in init_chrdev

[Thorsten Blum]

On Tue, Apr 28, 2026 at 10:37:49AM +0200, Eli Billauer wrote:
> Hello,
> 
> On 27/04/2026 19:37, Thorsten Blum wrote:
> 
> >   	if (!enumerate)
> > -		snprintf(unit->name, UNITNAMELEN, "%s", prefix);
> > +		strscpy(unit->name, prefix);
> >   	for (i = 0; enumerate; i++) {
> >   		snprintf(unit->name, UNITNAMELEN, "%s_%02d",
> 
> snprintf() is used deliberately for code clarity: It makes a simple visual
> contrast with the use of snprintf() three rows below. As this call takes
> place only once for each physical hardware device handled by the driver, the
> advantage in optimizing this function call is negligible.
> 
> And even though it's formally OK to use strscpy() with two arguments, and
> let the compilation machinery find out the length of the char array, I have
> to admit that it gives me the chills. Buffer overflow and that.

You probably mean strcpy()? strscpy() is safe regarding buffer overflow.

Re: [PATCH] char: xillybus: use strscpy() in init_chrdev

[Eli Billauer]

On 28/04/2026 10:55, Thorsten Blum wrote:
>> And even though it's formally OK to use strscpy() with two arguments, and
>> let the compilation machinery find out the length of the char array, I have
>> to admit that it gives me the chills. Buffer overflow and that.
> You probably mean strcpy()? strscpy() is safe regarding buffer overflow.

I meant strscpy(). We agree that it's formally safe to use, but I get a 
really bad feeling with not having the length of the buffer explicitly 
written out in the function call.

One could argue that it's better to let the machine figure out the 
length of the buffer instead of using UNITNAMELEN (because I'm a human 
and I can err), but in this specific case, UNITNAMELEN is relied upon 
anyhow a few rows later.

This is all about coding style. The main point is that I see this patch 
as a negligible optimization in CPU cycles coming with a slight 
obfuscation of the code's readability. A bit of "if it ain't broke, 
don't fix it".

Regards,
    Eli