| Message ID | 20230209094359.1549629-1-dan.scally@ideasonboard.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8488a831e0c4d59528d20713a14cb8958af15bfe |
| Headers | show |
| Series | usb: gadget: configfs: Fix set but not used variable warning | expand |
On Thu, Feb 09, 2023 at 09:43:59AM +0000, Daniel Scally wrote: > Fix a -Wunused-but-set-variable warning in gadget_string_s_store() A side comment below. ... > if (len > USB_MAX_STRING_LEN) > return -EINVAL; > > - ret = strscpy(string->string, page, size); > - return len; > + return strscpy(string->string, page, size); Do you need above check with strscpy()? You may supply the maximum length and negative error code from the strscpy() will indicate the cut.
Hi Andy On 10/02/2023 15:54, Andy Shevchenko wrote: > On Thu, Feb 09, 2023 at 09:43:59AM +0000, Daniel Scally wrote: >> Fix a -Wunused-but-set-variable warning in gadget_string_s_store() > A side comment below. > > ... > >> if (len > USB_MAX_STRING_LEN) >> return -EINVAL; >> >> - ret = strscpy(string->string, page, size); >> - return len; >> + return strscpy(string->string, page, size); > Do you need above check with strscpy()? You may supply the maximum length > and negative error code from the strscpy() will indicate the cut. > It would still copy the truncated string in that case though, correct? Seems cleaner to me to just fail and leave the string as-is, but I don't particularly mind either way.
On Fri, Feb 10, 2023 at 04:03:52PM +0000, Dan Scally wrote: > On 10/02/2023 15:54, Andy Shevchenko wrote: > > On Thu, Feb 09, 2023 at 09:43:59AM +0000, Daniel Scally wrote: > > > Fix a -Wunused-but-set-variable warning in gadget_string_s_store() > > A side comment below. ... > > > if (len > USB_MAX_STRING_LEN) > > > return -EINVAL; > > > - ret = strscpy(string->string, page, size); > > > - return len; > > > + return strscpy(string->string, page, size); > > Do you need above check with strscpy()? You may supply the maximum length > > and negative error code from the strscpy() will indicate the cut. > > > It would still copy the truncated string in that case though, correct? Seems > cleaner to me to just fail and leave the string as-is, but I don't > particularly mind either way. Good point. Yes, depending on the nature of the data we copy it (my proposal) may or may not be a good idea.
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 06a0b73e0546..b9f1136aa0a2 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -821,13 +821,11 @@ static ssize_t gadget_string_s_store(struct config_item *item, const char *page, { struct gadget_string *string = to_gadget_string(item); int size = min(sizeof(string->string), len + 1); - int ret; if (len > USB_MAX_STRING_LEN) return -EINVAL; - ret = strscpy(string->string, page, size); - return len; + return strscpy(string->string, page, size); } CONFIGFS_ATTR(gadget_string_, s);
Fix a -Wunused-but-set-variable warning in gadget_string_s_store() Fixes: 15a7cf8caabe ("usb: gadget: configfs: Support arbitrary string descriptors") Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com> --- drivers/usb/gadget/configfs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)