Re: [PATCH] USB: core: Use `krealloc()` in `usb_cache_string()`

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Mar 12, 2026 at 12:06:35AM +0100, Bence Csókás via B4 Relay wrote:
> From: Bence Csókás <bence98@sch.bme.hu>
> 
> Instead of "shrinking" the allocation by `kmalloc()`ing a new, smaller
> buffer, utilize `krealloc()` to shrink the existing allocation. This saves
> a `memcpy()`, as well as guards against `smallbuf` allocation failure.
> 
> Signed-off-by: Bence Csókás <bence98@sch.bme.hu>
> ---
> Using `krealloc()` makes this code from 2005 more readable as well as
> robust. Nested `if`s were also unrolled.

How is it more "robust" now?

> ---
>  drivers/usb/core/message.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

Same number of lines. Well, not quite, because I'm going to ask you to
remove the ?: stuff below...

> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index ea970ddf8879..dfe61d8b913b 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1005,7 +1005,7 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size)
>  }
>  EXPORT_SYMBOL_GPL(usb_string);
>  
> -/* one UTF-8-encoded 16-bit character has at most three bytes */
> +/* one 16-bit character, when UTF-8-encoded, has at most three bytes */

Why change this?

>  #define MAX_USB_STRING_SIZE (127 * 3 + 1)
>  
>  /**
> @@ -1026,17 +1026,17 @@ char *usb_cache_string(struct usb_device *udev, int index)
>  		return NULL;
>  
>  	buf = kmalloc(MAX_USB_STRING_SIZE, GFP_NOIO);
> -	if (buf) {
> -		len = usb_string(udev, index, buf, MAX_USB_STRING_SIZE);
> -		if (len > 0) {
> -			smallbuf = kmalloc(++len, GFP_NOIO);
> -			if (!smallbuf)
> -				return buf;
> -			memcpy(smallbuf, buf, len);
> -		}
> +	if (!buf)
> +		return NULL;
> +
> +	len = usb_string(udev, index, buf, MAX_USB_STRING_SIZE);
> +	if (len <= 0) {
>  		kfree(buf);
> +		return NULL;
>  	}
> -	return smallbuf;
> +
> +	smallbuf = krealloc(buf, len + 1, GFP_NOIO);
> +	return smallbuf ? : buf;

I hate ? : except where it can only be used (i.e. in function
arguments), so please spell it out exactly what you are doing here.

Also, how was this tested?

thanks,

greg k-h