| Message ID | 20230607215401.22563-1-ddiss@suse.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6b394dbb6469e438c537f84899402ffdb8fcbdbd |
| Headers | show |
| Series | [v2] usb: gadget: f_mass_storage: remove unnecessary open check | expand |
On Wed, Jun 07, 2023 at 11:54:02PM +0200, David Disseldorp wrote: > The fsg_lun_is_open() test can be eliminated and the code merged with > the preceding conditional, because the LUN won't be open if > cfg->filename wasn't set. Similarly, the error_lun label will never be > reached with an open lun (non-null filp) so remove the unnecessary > fsg_lun_close() call. > > Signed-off-by: David Disseldorp <ddiss@suse.de> > --- > Changes since v1: > - reword commit message, following Alan Stern's suggestions > > drivers/usb/gadget/function/f_mass_storage.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c > index 3a30feb47073f..da07e45ae6df5 100644 > --- a/drivers/usb/gadget/function/f_mass_storage.c > +++ b/drivers/usb/gadget/function/f_mass_storage.c > @@ -2857,7 +2857,7 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg, > const char **name_pfx) > { > struct fsg_lun *lun; > - char *pathbuf, *p; > + char *pathbuf = NULL, *p = "(no medium)"; > int rc = -ENOMEM; > > if (id >= ARRAY_SIZE(common->luns)) > @@ -2907,12 +2907,9 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg, > rc = fsg_lun_open(lun, cfg->filename); > if (rc) > goto error_lun; > - } > > - pathbuf = kmalloc(PATH_MAX, GFP_KERNEL); > - p = "(no medium)"; > - if (fsg_lun_is_open(lun)) { > p = "(error)"; > + pathbuf = kmalloc(PATH_MAX, GFP_KERNEL); > if (pathbuf) { > p = file_path(lun->filp, pathbuf, PATH_MAX); > if (IS_ERR(p)) > @@ -2931,7 +2928,6 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg, > error_lun: > if (device_is_registered(&lun->dev)) > device_unregister(&lun->dev); > - fsg_lun_close(lun); > common->luns[id] = NULL; > error_sysfs: > kfree(lun); Okay, now I see what's going on. Thanks for making this change. Reviewed-by: Alan Stern <stern@rowland.harvard.edu> Alan Stern
diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c index 3a30feb47073f..da07e45ae6df5 100644 --- a/drivers/usb/gadget/function/f_mass_storage.c +++ b/drivers/usb/gadget/function/f_mass_storage.c @@ -2857,7 +2857,7 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg, const char **name_pfx) { struct fsg_lun *lun; - char *pathbuf, *p; + char *pathbuf = NULL, *p = "(no medium)"; int rc = -ENOMEM; if (id >= ARRAY_SIZE(common->luns)) @@ -2907,12 +2907,9 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg, rc = fsg_lun_open(lun, cfg->filename); if (rc) goto error_lun; - } - pathbuf = kmalloc(PATH_MAX, GFP_KERNEL); - p = "(no medium)"; - if (fsg_lun_is_open(lun)) { p = "(error)"; + pathbuf = kmalloc(PATH_MAX, GFP_KERNEL); if (pathbuf) { p = file_path(lun->filp, pathbuf, PATH_MAX); if (IS_ERR(p)) @@ -2931,7 +2928,6 @@ int fsg_common_create_lun(struct fsg_common *common, struct fsg_lun_config *cfg, error_lun: if (device_is_registered(&lun->dev)) device_unregister(&lun->dev); - fsg_lun_close(lun); common->luns[id] = NULL; error_sysfs: kfree(lun);
The fsg_lun_is_open() test can be eliminated and the code merged with the preceding conditional, because the LUN won't be open if cfg->filename wasn't set. Similarly, the error_lun label will never be reached with an open lun (non-null filp) so remove the unnecessary fsg_lun_close() call. Signed-off-by: David Disseldorp <ddiss@suse.de> --- Changes since v1: - reword commit message, following Alan Stern's suggestions drivers/usb/gadget/function/f_mass_storage.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)