On Mon, Oct 14 2013, Andrzej Pietrasiewicz wrote:
> If cdrom flag is set ro flag is implied. Try setting the ro first, and
> only if it succeeds set the cdrom flag.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
> diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
> index 8bd5f2d..38cd4c4 100644
> --- a/drivers/usb/gadget/storage_common.c
> +++ b/drivers/usb/gadget/storage_common.c
> @@ -371,6 +371,20 @@ ssize_t fsg_show_removable(struct fsg_lun *curlun, char *buf)
> }
> EXPORT_SYMBOL(fsg_show_removable);
>
> +static ssize_t _fsg_store_ro(struct fsg_lun *curlun, bool ro)
Please add a comment saying that caller must hold filesem when calling
this function.
> +{
> + if (fsg_lun_is_open(curlun)) {
> + LDBG(curlun, "read-only status change prevented\n");
> + return -EBUSY;
> + }
> +
> + curlun->ro = ro;
> + curlun->initially_ro = ro;
> + LDBG(curlun, "read-only status set to %d\n", curlun->ro);
> +
> + return 0;
> +}
> +
> ssize_t fsg_store_ro(struct fsg_lun *curlun, struct rw_semaphore *filesem,
> const char *buf, size_t count)
> {
> @@ -386,15 +400,12 @@ ssize_t fsg_store_ro(struct fsg_lun *curlun, struct rw_semaphore *filesem,
> * backing file is closed.
> */
> down_read(filesem);
> - if (fsg_lun_is_open(curlun)) {
> - LDBG(curlun, "read-only status change prevented\n");
> - rc = -EBUSY;
> - } else {
> - curlun->ro = ro;
> - curlun->initially_ro = ro;
> - LDBG(curlun, "read-only status set to %d\n", curlun->ro);
> - rc = count;
> - }
> + rc = _fsg_store_ro(curlun, ro);
> + if (rc)
> + goto out;
> + rc = count;
if (!rc)
rc = count;
which will get rid of a goto and the out label. If you really want to
be adventurous, you could even do:
rc = _fsg_store_ro(curlun, ro) ?: count;
but that might be too much. ;)
> +
> +out:
> up_read(filesem);
> return rc;
> }
> @@ -459,9 +471,19 @@ ssize_t fsg_store_cdrom(struct fsg_lun *curlun, const char *buf, size_t count)
> if (ret)
> return ret;
>
> + down_read(filesem);
> + if (cdrom) {
> + ret = _fsg_store_ro(curlun, 1);
> + if (ret)
> + goto out;
> + }
> +
> curlun->cdrom = cdrom;
> + ret = count;
>
> - return count;
> +out:
Alternatively:
ret = cdrom ? _fsg_store_ro(curlun, true) : 0;
if (!ret) {
curlun->cdrom = cdrom;
ret = count;
}
which is somehow shorter and gets rid of the goto. And also, since
_fsg_store_ro takes bool as an argument, please pass true.
> + up_read(filesem);
> + return ret;
> }
> EXPORT_SYMBOL(fsg_store_cdrom);
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<mpn@xxxxxxxxxx>--<xmpp:mina86@xxxxxxxxxx>--ooO--(_)--Ooo--
Attachment:
signature.asc
Description: PGP signature
