Re: [PATCH v2 1/6] USB: add usb_make_subpath() for formatted device subpaths

[Alejandro Colomar <alx+linux-hardening@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3395 bytes --]

Hi Shihuang,

> Date: 2026-10-22 15:08:58+0800
> From: Shihuang Liu <shlomojune6@gmail.com>
>
> On Wed, Oct 21, 2026 at 03:17:56PM +0200, Alejandro Colomar wrote:
> > Hi Shihuang,
> > 
> > > Date: 2026-10-21 20:31:41+0800
> > > From: Shihuang Liu <shlomojune6@gmail.com>
> > >
> > > USB drivers often build device paths by calling usb_make_path() :and
> > > then appending a subpath. Several callers use the deprecated strlcat()
> > > API for this.
> > > 
> > > In preparation for removing the deprecated strlcat() API[1], add
> > > usb_make_subpath() to format the USB device path.
> > > 
> > > Link: https://github.com/KSPP/linux/issues/370 [1]
> > > Signed-off-by: Shihuang Liu <shlomojune6@gmail.com>
> > > ---
> > >  include/linux/usb.h | 29 +++++++++++++++++++++++++++++
> > >  1 file changed, 29 insertions(+)
> > > 
> > > diff --git a/include/linux/usb.h b/include/linux/usb.h
> > > index 1da4ad1610bca..c90ccea0cdbf6 100644
> > > --- a/include/linux/usb.h
> > > +++ b/include/linux/usb.h
> > > @@ -14,6 +14,7 @@
> > >  #include <linux/errno.h>        /* for -ENODEV */
> > >  #include <linux/delay.h>	/* for mdelay() */
> > >  #include <linux/interrupt.h>	/* for in_interrupt() */
> > > +#include <linux/stdarg.h>	/* for va_list */
> > >  #include <linux/list.h>		/* for struct list_head */
> > >  #include <linux/kref.h>		/* for struct kref */
> > >  #include <linux/device.h>	/* for struct device */
> > > @@ -987,6 +988,34 @@ static inline int usb_make_path(struct usb_device *dev, char *buf, size_t size)
> > >  	return (actual >= (int)size) ? -1 : actual;
> > >  }
> > >  
> > > +/**
> > > + * usb_make_subpath - returns a stable device path and formatted subpath
> > > + * @dev: the device whose path is being constructed
> > > + * @buf: where to put the string
> > > + * @size: how big is "buf"?
> > > + * @fmt: format string for the subpath, without the leading slash
> > > + * @...: arguments for @fmt
> > > + *
> > > + * Return: Length of the string (> 0) or negative if size was too small.
> > > + */
> > > +static inline __printf(4, 5)
> > > +int usb_make_subpath(struct usb_device *dev, char *buf, size_t size,
> > > +		     const char *fmt, ...)
> > > +{
> > > +	struct va_format vaf;
> > > +	va_list args;
> > > +	int actual;
> > > +
> > > +	va_start(args, fmt);
> > > +	vaf.fmt = fmt;
> > > +	vaf.va = &args;
> > > +	actual = snprintf(buf, size, "usb-%s-%s/%pV", dev->bus->bus_name,
> > > +			  dev->devpath, &vaf);
> > > +	va_end(args);
> > > +
> > > +	return (actual >= (int)size) ? -1 : actual;
> > 
> > Why are you open-coding usb_make_path() here instead of calling it?
> > 
> > 
> > Have a lovely day!
> > Alex
> > 
> Hi, Alex
> 
> With open-coding, a single snprintf() is enough, so we don't need to
> call usb_make_path() and then another function to append the suffix.

The number of function calls should be negligible compared to all the
work done by this function.  The readability and robustness is way more
important.


Have a lovely day!
Alex

> 
> Thanks,
> Shihuang
> > > +}
> > > +
> > >  /*-------------------------------------------------------------------------*/
> > >  
> > >  #define USB_DEVICE_ID_MATCH_DEVICE \
> > > -- 
> > > 2.43.0
> > > 
> > > 
> > 
> > -- 
> > <https://www.alejandro-colomar.es>

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]