[PATCH v3] Convert simple_strtoul to kstrtoul

[PATCH v3] Convert simple_strtoul to kstrtoul

[Navya Sri Nizamkari]

This patch fixes the checkpatch.pl warning:
simple_strtoul is obsolete, use kstrtoul instead.

When the string is not converted(parsing error), the strings pointed to by ep and parport[n] are the same. This parsing error results in kstrtoul returning a non-zero value.

Signed-off-by: Navya Sri Nizamkari <navyasri.tech@gmail.com>
---
Changes in v3:
   -Removed few changes made in previous commit which were introducing new checkpatch.pl warnings.

 drivers/staging/media/parport/bw-qcam.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/media/parport/bw-qcam.c b/drivers/staging/media/parport/bw-qcam.c
index 67b9da1..6c9b400 100644
--- a/drivers/staging/media/parport/bw-qcam.c
+++ b/drivers/staging/media/parport/bw-qcam.c
@@ -1106,10 +1106,8 @@ static int accept_bwqcam(struct parport *port)
 	if (parport[0] && strncmp(parport[0], "auto", 4) != 0) {
 		/* user gave parport parameters */
 		for (n = 0; n < MAX_CAMS && parport[n]; n++) {
-			char *ep;
 			unsigned long r;
-			r = simple_strtoul(parport[n], &ep, 0);
-			if (ep == parport[n]) {
+			if (kstrtoul(parport[n], 0, &r)) {
 				printk(KERN_ERR
 					"bw-qcam: bad port specifier \"%s\"\n",
 					parport[n]);
-- 
1.9.1

Re: [PATCH v3] Convert simple_strtoul to kstrtoul

[Navya Sri Nizamkari]

[-- Attachment #1.1: Type: text/plain, Size: 1755 bytes --]

Hi,
  Is this patch alright? Now, there are now additional warnings introduced 
by  the change I made.

On Thursday, February 19, 2015 at 11:40:18 PM UTC+5:30, Navya Sri Nizamkari 
wrote:
>
> This patch fixes the checkpatch.pl warning: 
> simple_strtoul is obsolete, use kstrtoul instead. 
>
> When the string is not converted(parsing error), the strings pointed to by 
> ep and parport[n] are the same. This parsing error results in kstrtoul 
> returning a non-zero value. 
>
> Signed-off-by: Navya Sri Nizamkari <navyasri.tech@gmail.com> 
> --- 
> Changes in v3: 
>    -Removed few changes made in previous commit which were introducing new 
> checkpatch.pl warnings. 
>
>  drivers/staging/media/parport/bw-qcam.c | 4 +--- 
>  1 file changed, 1 insertion(+), 3 deletions(-) 
>
> diff --git a/drivers/staging/media/parport/bw-qcam.c 
> b/drivers/staging/media/parport/bw-qcam.c 
> index 67b9da1..6c9b400 100644 
> --- a/drivers/staging/media/parport/bw-qcam.c 
> +++ b/drivers/staging/media/parport/bw-qcam.c 
> @@ -1106,10 +1106,8 @@ static int accept_bwqcam(struct parport *port) 
>          if (parport[0] && strncmp(parport[0], "auto", 4) != 0) { 
>                  /* user gave parport parameters */ 
>                  for (n = 0; n < MAX_CAMS && parport[n]; n++) { 
> -                        char *ep; 
>                          unsigned long r; 
> -                        r = simple_strtoul(parport[n], &ep, 0); 
> -                        if (ep == parport[n]) { 
> +                        if (kstrtoul(parport[n], 0, &r)) { 
>                                  printk(KERN_ERR 
>                                          "bw-qcam: bad port specifier 
> \"%s\"\n", 
>                                          parport[n]); 
> -- 
> 1.9.1 
>
>

[-- Attachment #1.2: Type: text/html, Size: 3602 bytes --]

Re: [Outreachy kernel] [PATCH v3] Convert simple_strtoul to kstrtoul

[Preeti U Murthy]

Hi Navya,

On 02/19/2015 11:41 PM, Navya Sri Nizamkari wrote:
> This patch fixes the checkpatch.pl warning:
> simple_strtoul is obsolete, use kstrtoul instead.
> 
> When the string is not converted(parsing error), the strings pointed to by ep and parport[n] are the same. This parsing error results in kstrtoul returning a non-zero value.

When you write changelogs restrict the length of the line to 80
characters, so that the patch looks uniformly aligned throughout.

On another note, the patch is fixing an issue that is not specific to ep
and parport[n]. It looks at replacing strtoul() with kstrtoul() in
places where there are checks for parsing error in string parsing.
Make the explanation more generic because its not specific to this file.
Ideally if any one else finds such a usage elsewhere, they can change it
the same way. Hence under such circumstances avoid using file specific
parameters in the changelog.

And try to link the usage of strtoul() with kstrtoul(); you replace
strtoul() with kstrtoul() in the error handling part here because both
check for similar errors while parsing strings. Ideally kstrtoul() error
check should be a superset of strtoul(). Else this patch would have been
wrong.

But hold onto these points for a similar cleanup on another driver. See
below.

> 
> Signed-off-by: Navya Sri Nizamkari <navyasri.tech@gmail.com>
> ---
> Changes in v3:
>    -Removed few changes made in previous commit which were introducing new checkpatch.pl warnings.
> 
>  drivers/staging/media/parport/bw-qcam.c | 4 +---

Refer to the mail that Arnd sent out. He has listed the drivers that are
not in mainline. So avoid sending patches against the following drivers,
since they will soon be removed from staging-next as well.

drivers/staging/line6/ (moved to sound/usb)
drivers/staging/media/parport/
drivers/staging/media/tlg2300/
drivers/staging/media/vino/

Regards
Preeti U Murthy

>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/media/parport/bw-qcam.c b/drivers/staging/media/parport/bw-qcam.c
> index 67b9da1..6c9b400 100644
> --- a/drivers/staging/media/parport/bw-qcam.c
> +++ b/drivers/staging/media/parport/bw-qcam.c
> @@ -1106,10 +1106,8 @@ static int accept_bwqcam(struct parport *port)
>  	if (parport[0] && strncmp(parport[0], "auto", 4) != 0) {
>  		/* user gave parport parameters */
>  		for (n = 0; n < MAX_CAMS && parport[n]; n++) {
> -			char *ep;
>  			unsigned long r;
> -			r = simple_strtoul(parport[n], &ep, 0);
> -			if (ep == parport[n]) {
> +			if (kstrtoul(parport[n], 0, &r)) {
>  				printk(KERN_ERR
>  					"bw-qcam: bad port specifier \"%s\"\n",
>  					parport[n]);
> 

Re: [Outreachy kernel] [PATCH v3] Convert simple_strtoul to kstrtoul

[navya sri nizamkari]

Hi,
I've made changes according to your feedback and sent in another
patch. Is i2o going to be removed too?
Thanks,
Navya.

On Fri, Feb 20, 2015 at 4:28 PM, Preeti U Murthy
<preeti@linux.vnet.ibm.com> wrote:
> Hi Navya,
>
> On 02/19/2015 11:41 PM, Navya Sri Nizamkari wrote:
>> This patch fixes the checkpatch.pl warning:
>> simple_strtoul is obsolete, use kstrtoul instead.
>>
>> When the string is not converted(parsing error), the strings pointed to by ep and parport[n] are the same. This parsing error results in kstrtoul returning a non-zero value.
>
> When you write changelogs restrict the length of the line to 80
> characters, so that the patch looks uniformly aligned throughout.
>
> On another note, the patch is fixing an issue that is not specific to ep
> and parport[n]. It looks at replacing strtoul() with kstrtoul() in
> places where there are checks for parsing error in string parsing.
> Make the explanation more generic because its not specific to this file.
> Ideally if any one else finds such a usage elsewhere, they can change it
> the same way. Hence under such circumstances avoid using file specific
> parameters in the changelog.
>
> And try to link the usage of strtoul() with kstrtoul(); you replace
> strtoul() with kstrtoul() in the error handling part here because both
> check for similar errors while parsing strings. Ideally kstrtoul() error
> check should be a superset of strtoul(). Else this patch would have been
> wrong.
>
> But hold onto these points for a similar cleanup on another driver. See
> below.
>
>>
>> Signed-off-by: Navya Sri Nizamkari <navyasri.tech@gmail.com>
>> ---
>> Changes in v3:
>>    -Removed few changes made in previous commit which were introducing new checkpatch.pl warnings.
>>
>>  drivers/staging/media/parport/bw-qcam.c | 4 +---
>
> Refer to the mail that Arnd sent out. He has listed the drivers that are
> not in mainline. So avoid sending patches against the following drivers,
> since they will soon be removed from staging-next as well.
>
> drivers/staging/line6/ (moved to sound/usb)
> drivers/staging/media/parport/
> drivers/staging/media/tlg2300/
> drivers/staging/media/vino/
>
> Regards
> Preeti U Murthy
>
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/media/parport/bw-qcam.c b/drivers/staging/media/parport/bw-qcam.c
>> index 67b9da1..6c9b400 100644
>> --- a/drivers/staging/media/parport/bw-qcam.c
>> +++ b/drivers/staging/media/parport/bw-qcam.c
>> @@ -1106,10 +1106,8 @@ static int accept_bwqcam(struct parport *port)
>>       if (parport[0] && strncmp(parport[0], "auto", 4) != 0) {
>>               /* user gave parport parameters */
>>               for (n = 0; n < MAX_CAMS && parport[n]; n++) {
>> -                     char *ep;
>>                       unsigned long r;
>> -                     r = simple_strtoul(parport[n], &ep, 0);
>> -                     if (ep == parport[n]) {
>> +                     if (kstrtoul(parport[n], 0, &r)) {
>>                               printk(KERN_ERR
>>                                       "bw-qcam: bad port specifier \"%s\"\n",
>>                                       parport[n]);
>>
>

Re: [Outreachy kernel] Re: [PATCH v3] Convert simple_strtoul to kstrtoul

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2830 bytes --]

On Fri, 20 Feb 2015, Navya Sri Nizamkari wrote:

> Hi,
>   Is this patch alright? Now, there are now additional warnings introduced
> by  the change I made.

What warnings?

julia

>
> On Thursday, February 19, 2015 at 11:40:18 PM UTC+5:30, Navya Sri Nizamkari
> wrote:
>       This patch fixes the checkpatch.pl warning:
>       simple_strtoul is obsolete, use kstrtoul instead.
>
>       When the string is not converted(parsing error), the strings
>       pointed to by ep and parport[n] are the same. This parsing error
>       results in kstrtoul returning a non-zero value.
>
>       Signed-off-by: Navya Sri Nizamkari <navyasri.tech@gmail.com>
>       ---
>       Changes in v3:
>          -Removed few changes made in previous commit which were
>       introducing new checkpatch.pl warnings.
>
>        drivers/staging/media/parport/bw-qcam.c | 4 +---
>        1 file changed, 1 insertion(+), 3 deletions(-)
>
>       diff --git a/drivers/staging/media/parport/bw-qcam.c
>       b/drivers/staging/media/parport/bw-qcam.c
>       index 67b9da1..6c9b400 100644
>       --- a/drivers/staging/media/parport/bw-qcam.c
>       +++ b/drivers/staging/media/parport/bw-qcam.c
>       @@ -1106,10 +1106,8 @@ static int accept_bwqcam(struct parport
>       *port)
>                if (parport[0] && strncmp(parport[0], "auto", 4) != 0)
>       {
>                        /* user gave parport parameters */
>                        for (n = 0; n < MAX_CAMS && parport[n]; n++) {
>       -                        char *ep;
>                                unsigned long r;
>       -                        r = simple_strtoul(parport[n], &ep, 0);
>       -                        if (ep == parport[n]) {
>       +                        if (kstrtoul(parport[n], 0, &r)) {
>                                        printk(KERN_ERR
>                                                "bw-qcam: bad port
>       specifier \"%s\"\n",
>                                                parport[n]);
>       --
>       1.9.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/c4bae1a8-b908-47ae-b7e5-
> e293248a7691%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

Re: [Outreachy kernel] Re: [PATCH v3] Convert simple_strtoul to kstrtoul

[Navya Sri Nizamkari]

[-- Attachment #1.1: Type: text/plain, Size: 636 bytes --]



On Friday, February 20, 2015 at 11:07:58 PM UTC+5:30, Julia Lawall wrote:
>
> On Fri, 20 Feb 2015, Navya Sri Nizamkari wrote: 
>
> > Hi, 
> >   Is this patch alright? Now, there are now additional warnings 
> introduced 
> > by  the change I made. 
>
> What warnings? 
>
> julia 
>
>
> > -                        r = simple_strtoul(parport[n], &ep, 0); 
> > -                        if (ep == parport[n]) { 
> > +                        if (kstrtoul(parport[n],0,&r)) { 
>
> In the first patch I sent, there was no space after the ',' in kstrtoul. I 
was talking about the new warnings this created.So, I sent a another one.

Navya
 

[-- Attachment #1.2: Type: text/html, Size: 1229 bytes --]

Re: [Outreachy kernel] Re: [PATCH v3] Convert simple_strtoul to kstrtoul

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 884 bytes --]

On Fri, 20 Feb 2015, Navya Sri Nizamkari wrote:

> 
> 
> On Friday, February 20, 2015 at 11:07:58 PM UTC+5:30, Julia Lawall wrote:
>       On Fri, 20 Feb 2015, Navya Sri Nizamkari wrote:
> 
>       > Hi,
>       >   Is this patch alright? Now, there are now additional
>       warnings introduced
>       > by  the change I made.
> 
>       What warnings?
> 
>       julia
> 
> 
>       > -                        r = simple_strtoul(parport[n], &ep,
>       0);
>       > -                        if (ep == parport[n]) {
>       > +                        if (kstrtoul(parport[n],0,&r)) {
> 
> In the first patch I sent, there was no space after the ',' in kstrtoul. I
> was talking about the new warnings this created.So, I sent a another one.

OK.  Patches should not generate new checkpatch warnings.

julia