linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kstrtox: convert net/bluetooth/
@ 2011-04-02 11:19 Alexey Dobriyan
  2011-04-04 22:00 ` Gustavo F. Padovan
  0 siblings, 1 reply; 4+ messages in thread
From: Alexey Dobriyan @ 2011-04-02 11:19 UTC (permalink / raw)
  To: marcel; +Cc: linux-bluetooth

Convert from strict_strto*() interfaces to kstrto*() interfaces.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 net/bluetooth/hci_sysfs.c |   37 +++++++++++++++++--------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -277,11 +277,12 @@ static ssize_t show_idle_timeout(struct device *dev, struct device_attribute *at
 static ssize_t store_idle_timeout(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct hci_dev *hdev = dev_get_drvdata(dev);
-	unsigned long val;
-
-	if (strict_strtoul(buf, 0, &val) < 0)
-		return -EINVAL;
+	unsigned int val;
+	int rv;
 
+	rv = kstrtouint(buf, 0, &val);
+	if (rv < 0)
+		return rv;
 	if (val != 0 && (val < 500 || val > 3600000))
 		return -EINVAL;
 
@@ -299,15 +300,13 @@ static ssize_t show_sniff_max_interval(struct device *dev, struct device_attribu
 static ssize_t store_sniff_max_interval(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct hci_dev *hdev = dev_get_drvdata(dev);
-	unsigned long val;
-
-	if (strict_strtoul(buf, 0, &val) < 0)
-		return -EINVAL;
-
-	if (val < 0x0002 || val > 0xFFFE || val % 2)
-		return -EINVAL;
+	u16 val;
+	int rv;
 
-	if (val < hdev->sniff_min_interval)
+	rv = kstrtou16(buf, 0, &val);
+	if (rv < 0)
+		return rv;
+	if (val == 0 || val % 2 || val < hdev->sniff_min_interval)
 		return -EINVAL;
 
 	hdev->sniff_max_interval = val;
@@ -324,15 +323,13 @@ static ssize_t show_sniff_min_interval(struct device *dev, struct device_attribu
 static ssize_t store_sniff_min_interval(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct hci_dev *hdev = dev_get_drvdata(dev);
-	unsigned long val;
-
-	if (strict_strtoul(buf, 0, &val) < 0)
-		return -EINVAL;
-
-	if (val < 0x0002 || val > 0xFFFE || val % 2)
-		return -EINVAL;
+	u16 val;
+	int rv;
 
-	if (val > hdev->sniff_max_interval)
+	rv = kstrtou16(buf, 0, &val);
+	if (rv < 0)
+		return rv;
+	if (val == 0 || val % 2 || val > hdev->sniff_max_interval)
 		return -EINVAL;
 
 	hdev->sniff_min_interval = val;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] kstrtox: convert net/bluetooth/
  2011-04-02 11:19 [PATCH] kstrtox: convert net/bluetooth/ Alexey Dobriyan
@ 2011-04-04 22:00 ` Gustavo F. Padovan
  2011-04-05 12:33   ` Alexey Dobriyan
  0 siblings, 1 reply; 4+ messages in thread
From: Gustavo F. Padovan @ 2011-04-04 22:00 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: marcel, linux-bluetooth

Hi Alexey,

* Alexey Dobriyan <adobriyan@gmail.com> [2011-04-02 14:19:41 +0300]:

> Convert from strict_strto*() interfaces to kstrto*() interfaces.
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
> 
>  net/bluetooth/hci_sysfs.c |   37 +++++++++++++++++--------------------
>  1 file changed, 17 insertions(+), 20 deletions(-)
> 
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -277,11 +277,12 @@ static ssize_t show_idle_timeout(struct device *dev, struct device_attribute *at
>  static ssize_t store_idle_timeout(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
>  {
>  	struct hci_dev *hdev = dev_get_drvdata(dev);
> -	unsigned long val;
> -
> -	if (strict_strtoul(buf, 0, &val) < 0)
> -		return -EINVAL;
> +	unsigned int val;
> +	int rv;
>  
> +	rv = kstrtouint(buf, 0, &val);
> +	if (rv < 0)
> +		return rv;

Add a empty line here, please.

>  	if (val != 0 && (val < 500 || val > 3600000))
>  		return -EINVAL;
>  
> @@ -299,15 +300,13 @@ static ssize_t show_sniff_max_interval(struct device *dev, struct device_attribu
>  static ssize_t store_sniff_max_interval(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
>  {
>  	struct hci_dev *hdev = dev_get_drvdata(dev);
> -	unsigned long val;
> -
> -	if (strict_strtoul(buf, 0, &val) < 0)
> -		return -EINVAL;
> -
> -	if (val < 0x0002 || val > 0xFFFE || val % 2)
> -		return -EINVAL;
> +	u16 val;
> +	int rv;
>  
> -	if (val < hdev->sniff_min_interval)
> +	rv = kstrtou16(buf, 0, &val);
> +	if (rv < 0)
> +		return rv;
> +	if (val == 0 || val % 2 || val < hdev->sniff_min_interval)
>  		return -EINVAL;

Why are you changing other things besides the string conversions? The checks
for val should stay the same.

>  
>  	hdev->sniff_max_interval = val;
> @@ -324,15 +323,13 @@ static ssize_t show_sniff_min_interval(struct device *dev, struct device_attribu
>  static ssize_t store_sniff_min_interval(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
>  {
>  	struct hci_dev *hdev = dev_get_drvdata(dev);
> -	unsigned long val;
> -
> -	if (strict_strtoul(buf, 0, &val) < 0)
> -		return -EINVAL;
> -
> -	if (val < 0x0002 || val > 0xFFFE || val % 2)
> -		return -EINVAL;
> +	u16 val;
> +	int rv;
>  
> -	if (val > hdev->sniff_max_interval)
> +	rv = kstrtou16(buf, 0, &val);
> +	if (rv < 0)
> +		return rv;
> +	if (val == 0 || val % 2 || val > hdev->sniff_max_interval)
>  		return -EINVAL;

Same problem here.


-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] kstrtox: convert net/bluetooth/
  2011-04-04 22:00 ` Gustavo F. Padovan
@ 2011-04-05 12:33   ` Alexey Dobriyan
  2011-04-05 16:38     ` Gustavo F. Padovan
  0 siblings, 1 reply; 4+ messages in thread
From: Alexey Dobriyan @ 2011-04-05 12:33 UTC (permalink / raw)
  To: Alexey Dobriyan, marcel, linux-bluetooth; +Cc: Gustavo F. Padovan

On Tue, Apr 5, 2011 at 1:00 AM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:

>> @@ -299,15 +300,13 @@ static ssize_t show_sniff_max_interval(struct device *dev, struct device_attribu
>>  static ssize_t store_sniff_max_interval(struct device *dev, struct device_attribute *attr, const char *buf, size_t count)
>>  {
>>       struct hci_dev *hdev = dev_get_drvdata(dev);
>> -     unsigned long val;
>> -
>> -     if (strict_strtoul(buf, 0, &val) < 0)
>> -             return -EINVAL;
>> -
>> -     if (val < 0x0002 || val > 0xFFFE || val % 2)
>> -             return -EINVAL;
>> +     u16 val;
>> +     int rv;
>>
>> -     if (val < hdev->sniff_min_interval)
>> +     rv = kstrtou16(buf, 0, &val);
>> +     if (rv < 0)
>> +             return rv;
>> +     if (val == 0 || val % 2 || val < hdev->sniff_min_interval)
>>               return -EINVAL;
>
> Why are you changing other things besides the string conversions?

Because this is not mindless s/foo/bar/g conversion.

> The checks for val should stay the same.

No, they should not. kstrtou16() takes care of [0x0000, 0xFFFF] interval,
then you check for even values, then you have only 0 to ban.

WIth previous checks, say, "val > 0xFFFE" is not necessary,
kstrtou16 + "val % 2" do it.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] kstrtox: convert net/bluetooth/
  2011-04-05 12:33   ` Alexey Dobriyan
@ 2011-04-05 16:38     ` Gustavo F. Padovan
  0 siblings, 0 replies; 4+ messages in thread
From: Gustavo F. Padovan @ 2011-04-05 16:38 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: marcel, linux-bluetooth

Hi Alexey,

* Alexey Dobriyan <adobriyan@gmail.com> [2011-04-05 15:33:47 +0300]:

> On Tue, Apr 5, 2011 at 1:00 AM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
>=20
> >> @@ -299,15 +300,13 @@ static ssize_t show_sniff_max_interval(struct de=
vice *dev, struct device_attribu
> >> =A0static ssize_t store_sniff_max_interval(struct device *dev, struct =
device_attribute *attr, const char *buf, size_t count)
> >> =A0{
> >> =A0 =A0 =A0 struct hci_dev *hdev =3D dev_get_drvdata(dev);
> >> - =A0 =A0 unsigned long val;
> >> -
> >> - =A0 =A0 if (strict_strtoul(buf, 0, &val) < 0)
> >> - =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> >> -
> >> - =A0 =A0 if (val < 0x0002 || val > 0xFFFE || val % 2)
> >> - =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> >> + =A0 =A0 u16 val;
> >> + =A0 =A0 int rv;
> >>
> >> - =A0 =A0 if (val < hdev->sniff_min_interval)
> >> + =A0 =A0 rv =3D kstrtou16(buf, 0, &val);
> >> + =A0 =A0 if (rv < 0)
> >> + =A0 =A0 =A0 =A0 =A0 =A0 return rv;
> >> + =A0 =A0 if (val =3D=3D 0 || val % 2 || val < hdev->sniff_min_interva=
l)
> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> >
> > Why are you changing other things besides the string conversions?
>=20
> Because this is not mindless s/foo/bar/g conversion.
>=20
> > The checks for val should stay the same.
>=20
> No, they should not. kstrtou16() takes care of [0x0000, 0xFFFF] interval,
> then you check for even values, then you have only 0 to ban.
>=20
> WIth previous checks, say, "val > 0xFFFE" is not necessary,
> kstrtou16 + "val % 2" do it.

Ok, I agree, I didn't noted that at in a first review. Patch is now applied.
Thanks.


--=20
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-04-05 16:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-02 11:19 [PATCH] kstrtox: convert net/bluetooth/ Alexey Dobriyan
2011-04-04 22:00 ` Gustavo F. Padovan
2011-04-05 12:33   ` Alexey Dobriyan
2011-04-05 16:38     ` Gustavo F. Padovan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).