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

[Preeti U Murthy <preeti@linux.vnet.ibm.com>]

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]);
>