diff mbox

[OPW,kernel] Doubt in kstrtoul patch in kobjects.c

Message ID CAJzd_-TJrW=HzCcT-E3_=+Rju-+MyF3bggAY386WP6KZbvxFcA@mail.gmail.com
State New, archived
Headers show

Commit Message

Ashvini Varatharaj Oct. 20, 2013, 4:48 a.m. UTC
I have replaced simple_strtoul with kstrtoul in kobjects.c and submitted a
patch. I have removed the 'temp' variable since it is of no use in
kstrtoul.I want to know if I can eliminate the temp variable which will
involve eliminating many more variables further.Can you please help me? The
patch I have submitted is as follows:

Fix checkpatch warning: WARNING: simple_strtoul is obsolete, use
kstrtoul instead

Signed-off-by: Ashvini Varatharaj <ashvinivaratharaj@gmail.com>
---
 drivers/staging/speakup/kobjects.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

                        temp++;
@@ -781,7 +783,8 @@ static ssize_t message_store_helper(const char *buf,
size_t count,
                        continue;
                }

-               index = simple_strtoul(cp, &temp, 10);
+               if (kstrtoul(cp, 10, &index) != 0)
+                       pr_warn("Overflow or parsing error has occured");

                while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
                        temp++;

Thankyou

*Ashvini Varatharaj *

Comments

Waskiewicz Jr, Peter P Oct. 20, 2013, 8:52 a.m. UTC | #1
On Sun, 2013-10-20 at 10:18 +0530, Ashvini Varatharaj wrote:
> 
> 
> I have replaced simple_strtoul with kstrtoul in kobjects.c and
> submitted a patch. I have removed the 'temp' variable since it is of
> no use in kstrtoul.I want to know if I can eliminate the temp variable
> which will involve eliminating many more variables further.Can you
> please help me? The patch I have submitted is as follows:

Can you please resend the patch using git?  It appears the original
message was sent as HTML, so it came through not in a format I can
easily parse with my mail client.

In practice, if you can eliminate variables and still keep the same
functionality, that's a big plus.  Reducing stack size is always a good
thing.

-PJ

> 
> Fix checkpatch warning: WARNING: simple_strtoul is obsolete, use
> kstrtoul instead
> 
> Signed-off-by: Ashvini Varatharaj <ashvinivaratharaj@gmail.com>
> ---
>  drivers/staging/speakup/kobjects.c |   17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/speakup/kobjects.c
> b/drivers/staging/speakup/kobjects.c
> index e2f597e..3b9482d 100644
> --- a/drivers/staging/speakup/kobjects.c
> +++ b/drivers/staging/speakup/kobjects.c
> @@ -153,12 +153,14 @@ static ssize_t chars_chartab_store(struct
> kobject *kobj,
>                         continue;
>                 }
> 
> -               index = simple_strtoul(cp, &temp, 10);
> -               if (index > 255) {
> -                       rejected++;
> -                       cp = linefeed + 1;
> -                       continue;
> -               }
> +               if (kstrtoul(cp, 10, &index) == 0) {
> +                       if (index > 255) {
> +                               rejected++;
> +                               cp = linefeed + 1;
> +                               continue;
> +                       }
> +               } else
> +                       pr_warn("Overflow or parsing error has
> occured");
> 
>                 while ((temp < linefeed) && (*temp == ' ' || *temp ==
> '\t'))
>                         temp++;
> @@ -781,7 +783,8 @@ static ssize_t message_store_helper(const char
> *buf, size_t count,
>                         continue;
>                 }
> 
> -               index = simple_strtoul(cp, &temp, 10);
> +               if (kstrtoul(cp, 10, &index) != 0)
> +                       pr_warn("Overflow or parsing error has
> occured");
> 
>                 while ((temp < linefeed) && (*temp == ' ' || *temp ==
> '\t'))
>                         temp++;
> 
> 
> Thankyou
> 
> 
> Ashvini Varatharaj 
> 
>
Waskiewicz Jr, Peter P Oct. 20, 2013, 8:53 a.m. UTC | #2
On Sun, 2013-10-20 at 08:52 +0000, Waskiewicz Jr, Peter P wrote:
> On Sun, 2013-10-20 at 10:18 +0530, Ashvini Varatharaj wrote:
> > 
> > 
> > I have replaced simple_strtoul with kstrtoul in kobjects.c and
> > submitted a patch. I have removed the 'temp' variable since it is of
> > no use in kstrtoul.I want to know if I can eliminate the temp variable
> > which will involve eliminating many more variables further.Can you
> > please help me? The patch I have submitted is as follows:
> 
> Can you please resend the patch using git?  It appears the original
> message was sent as HTML, so it came through not in a format I can
> easily parse with my mail client.

Ugh, I see that you did send it already via git.  This reply that you
sent with that patch was the HTML-formatted one, hence my confusion.
Sorry for the noise (I'll go review the patch now).

-PJ
diff mbox

Patch

diff --git a/drivers/staging/speakup/kobjects.c b/drivers/staging/speakup/
kobjects.c
index e2f597e..3b9482d 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -153,12 +153,14 @@  static ssize_t chars_chartab_store(struct kobject
*kobj,
                        continue;
                }

-               index = simple_strtoul(cp, &temp, 10);
-               if (index > 255) {
-                       rejected++;
-                       cp = linefeed + 1;
-                       continue;
-               }
+               if (kstrtoul(cp, 10, &index) == 0) {
+                       if (index > 255) {
+                               rejected++;
+                               cp = linefeed + 1;
+                               continue;
+                       }
+               } else
+                       pr_warn("Overflow or parsing error has occured");

                while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))