Prev: [RFC PATCH] cfq-iosced: Implement IOPS mode and group_idle tunable V3
Next: [PATCH 1/3] DMAENGINE: generic slave channel control v2
From: Greg KH on 21 Jul 2010 15:20
On Thu, Jul 15, 2010 at 12:07:20PM +0300, Andy Shevchenko wrote:
> Bring a strict way to get the 'ro' parameter from the user.
> The idea was discussed in LKML [1].
>
> [1] http://lkml.org/lkml/2010/7/13/143

Care to provide a bit more information as to why this is being changed
instead of just a link to an old email?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on 22 Jul 2010 10:10
On Thu, 22 Jul 2010, Andy Shevchenko wrote:

> Bring a strict way to get the 'ro' parameter from the user.
>
> The patch followed by this one adds another boolean parameter. To be consistent
> Michał Nazarewicz proposed to use simple_strtol() in both cases (correspondend
> discussion in LKML [1]). Due to simple_strtol() doesn't return error in a good
> way and we have a boolean parameter the strict_strtoul() is used.
>
> [1] http://lkml.org/lkml/2010/7/14/169
>
> Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko(a)nokia.com>
> Cc: Alan Stern <stern(a)rowland.harvard.edu>
> Cc: David Brownell <dbrownell(a)users.sourceforge.net>
> Cc: Greg Kroah-Hartman <gregkh(a)suse.de>
> Cc: linux-usb(a)vger.kernel.org
> ---
> drivers/usb/gadget/storage_common.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/storage_common.c b/drivers/usb/gadget/storage_common.c
> index 04c462f..7123929 100644
> --- a/drivers/usb/gadget/storage_common.c
> +++ b/drivers/usb/gadget/storage_common.c
> @@ -723,9 +723,9 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
> ssize_t rc = count;
> struct fsg_lun *curlun = fsg_lun_from_dev(dev);
> struct rw_semaphore *filesem = dev_get_drvdata(dev);
> - int i;
> + unsigned long ro;
>
> - if (sscanf(buf, "%d", &i) != 1)
> + if (strict_strtoul(buf, 2, &ro))
> return -EINVAL;
>
> /* Allow the write-enable status to change only while the backing file
> @@ -735,8 +735,8 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
> LDBG(curlun, "read-only status change prevented\n");
> rc = -EBUSY;
> } else {
> - curlun->ro = !!i;
> - curlun->initially_ro = !!i;
> + curlun->ro = ro;
> + curlun->initially_ro = ro;
> LDBG(curlun, "read-only status set to %d\n", curlun->ro);
> }
> up_read(filesem);

Acked-by: Alan Stern <stern(a)rowland.harvard.edu>

Strictly speaking, this changes the behavior when a non-binary value is
specified. For example, if somebody were to write "10" then the old
code would set the value to True and the new code would set the value
to False -- and neither would reject it as they probably should.
That's okay; people who write ambiguous or invalid values deserve what
they get.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Michał Nazarewicz on 22 Jul 2010 10:20
> On Thu, 22 Jul 2010, Andy Shevchenko wrote:
>> @@ -735,8 +735,8 @@ static ssize_t fsg_store_ro(struct device *dev, struct device_attribute *attr,
>> LDBG(curlun, "read-only status change prevented\n");
>> rc = -EBUSY;
>> } else {
>> - curlun->ro = !!i;
>> - curlun->initially_ro = !!i;
>> + curlun->ro = ro;
>> + curlun->initially_ro = ro;
>> LDBG(curlun, "read-only status set to %d\n", curlun->ro);
>> }
>> up_read(filesem);

On Thu, 22 Jul 2010 16:07:14 +0200, Alan Stern <stern(a)rowland.harvard.edu> wrote:
> Strictly speaking, this changes the behavior when a non-binary value is
> specified. For example, if somebody were to write "10" then the old
> code would set the value to True and the new code would set the value
> to False -- and neither would reject it as they probably should.
> That's okay; people who write ambiguous or invalid values deserve what
> they get.

If anyone considers that an issue, it would be solved by:

+ curlun->ro = !!ro;
+ curlun->initially_ro = !!ro;

The "!!" where there for that purpose in the first place.

--
Best regards, _ _
| Humble Liege of Serenely Enlightened Majesty of o' \,=./ `o
| Computer Science, Michał "mina86" Nazarewicz (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: [RFC PATCH] cfq-iosced: Implement IOPS mode and group_idle tunable V3
Next: [PATCH 1/3] DMAENGINE: generic slave channel control v2