Re: [PATCH] USB: core: Use `krealloc()` in `usb_cache_string()`
From: Greg Kroah-Hartman
Date: Thu Mar 12 2026 - 01:02:45 EST
On Thu, Mar 12, 2026 at 12:06:35AM +0100, Bence Csókás via B4 Relay wrote:
> From: Bence Csókás <bence98@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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