Re: [PATCH 1/3] usb: Rename temp variable "config" to "val" in the set_avoid_reset_quirk()
On Mon, Jul 30, 2012 at 10:33:27AM +0200, Bjørn Mork wrote:
> Lan Tianyu writes:
>
> > - if (sscanf(buf, "%d", &config) != 1 || config < 0 || config > 1)
> > + if (sscanf(buf, "%d", &val) != 1 || val < 0 || val > 1)
> > return -EINVAL;
>
> Not directly related to this patch, but a question I started wondering
> about recently: Is there some generic guideline wrt parsing boolean
> flags in sysfs? If not, shouldn't there be?
>
> I see a lot of different approaches implementing this basic function.
> Personally I would prefer if they all did something like the
> set_usb2_hardware_lpm in drivers/usb/core/sysfs.c:
>
> static ssize_t
> set_usb2_hardware_lpm(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct usb_device *udev = to_usb_device(dev);
> bool value;
> int ret;
>
> usb_lock_device(udev);
>
> ret = strtobool(buf, &value);
>
> if (!ret)
> ret = usb_set_usb2_hardware_lpm(udev, value);
>
> usb_unlock_device(udev);
>
> if (!ret)
> return count;
>
> return ret;
> }
>
>
>
> Using strtobool() to allow "Y", "yes", "1" etc makes a nice user
> interface IMHO. Unless of course the variable is a true integer which
> only happens to currently allow 0 and 1, but may be extended with more
> values later.
Yes, using that function should be encouraged.
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: Rename temp variable "config" to "val" in the set_avoid_reset_quirk()
On Mon, 30 Jul 2012, Bjørn Mork wrote: > Lan Tianyu writes: > > > - if (sscanf(buf, "%d", &config) != 1 || config < 0 || config > 1) > > + if (sscanf(buf, "%d", &val) != 1 || val < 0 || val > 1) > > return -EINVAL; > > Not directly related to this patch, but a question I started wondering > about recently: Is there some generic guideline wrt parsing boolean > flags in sysfs? If not, shouldn't there be? Agreed; this area is ripe for kernel janitors. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: Rename temp variable "config" to "val" in the set_avoid_reset_quirk()
On Mon, 30 Jul 2012 10:33:27 +0200, Bjørn Mork wrote: Lan Tianyu writes: Not directly related to this patch, but a question I started wondering about recently: Is there some generic guideline wrt parsing boolean flags in sysfs? If not, shouldn't there be? I see a lot of different approaches implementing this basic function. Personally I would prefer if they all did something like the set_usb2_hardware_lpm in drivers/usb/core/sysfs.c: [...] ret = strtobool(buf, &value); [...] Using strtobool() to allow "Y", "yes", "1" etc makes a nice user interface IMHO. Unless of course the variable is a true integer which only happens to currently allow 0 and 1, but may be extended with more values later. I'd say that if someone is handling sysfs directly, he has burn in his brain that 0 is false and 1 is true. ;) (Shell complicates things a little, but still I feel that for sysfs users the connection is obvious.) Because of that, I stick to kstrtouint(). Also, I don't like how strtobool() treats "01" as a false value. -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: Rename temp variable "config" to "val" in the set_avoid_reset_quirk()
On Mon, 30 Jul 2012, Lan Tianyu wrote: > In USB, the word "config" already has a separate meaning. So it will > cause confusion if use "config" as variable's name for other purposes. > This patch is to convert the "config" to "val" > > Signed-off-by: Lan Tianyu Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] usb: Rename temp variable "config" to "val" in the set_avoid_reset_quirk()
Lan Tianyu writes:
> - if (sscanf(buf, "%d", &config) != 1 || config < 0 || config > 1)
> + if (sscanf(buf, "%d", &val) != 1 || val < 0 || val > 1)
> return -EINVAL;
Not directly related to this patch, but a question I started wondering
about recently: Is there some generic guideline wrt parsing boolean
flags in sysfs? If not, shouldn't there be?
I see a lot of different approaches implementing this basic function.
Personally I would prefer if they all did something like the
set_usb2_hardware_lpm in drivers/usb/core/sysfs.c:
static ssize_t
set_usb2_hardware_lpm(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct usb_device *udev = to_usb_device(dev);
bool value;
int ret;
usb_lock_device(udev);
ret = strtobool(buf, &value);
if (!ret)
ret = usb_set_usb2_hardware_lpm(udev, value);
usb_unlock_device(udev);
if (!ret)
return count;
return ret;
}
Using strtobool() to allow "Y", "yes", "1" etc makes a nice user
interface IMHO. Unless of course the variable is a true integer which
only happens to currently allow 0 and 1, but may be extended with more
values later.
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] usb: Rename temp variable "config" to "val" in the set_avoid_reset_quirk()
In USB, the word "config" already has a separate meaning. So it will
cause confusion if use "config" as variable's name for other purposes.
This patch is to convert the "config" to "val"
Signed-off-by: Lan Tianyu
---
Patchset is based on usb-next commit e387ef5c47ddeaeaa3cbdc54424cdb7a28dae2c0.
---
drivers/usb/core/sysfs.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index 682e825..d6c49d9 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -204,12 +204,12 @@ set_avoid_reset_quirk(struct device *dev, struct
device_attribute *attr,
const char *buf, size_t count)
{
struct usb_device *udev = to_usb_device(dev);
- int config;
+ int val;
- if (sscanf(buf, "%d", &config) != 1 || config < 0 || config > 1)
+ if (sscanf(buf, "%d", &val) != 1 || val < 0 || val > 1)
return -EINVAL;
usb_lock_device(udev);
- if (config)
+ if (val)
udev->quirks |= USB_QUIRK_RESET_MORPHS;
else
udev->quirks &= ~USB_QUIRK_RESET_MORPHS;
--
1.7.6.rc2.8.g28eb
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
