2015-12-03 08:06:20

by Corentin Labbe

Subject: [PATCH v3 0/1] atm: solos-pci: Replace simple_strtol by kstrtoint

Hello

Change since v2
- Invert a test logic

Change since v1
- Always return error code from kstrtox.

LABBE Corentin (1):
atm: solos-pci: Replace simple_strtol by kstrtoint

drivers/atm/solos-pci.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)

--
2.4.10

2015-12-03 08:06:22

by Corentin Labbe

Subject: [PATCH v3 1/1] atm: solos-pci: Replace simple_strtol by kstrtoint

The simple_strtol function is obsolete.
This patch replace it by kstrtoint.
This will simplify code, since some error case not handled by
simple_strtol are handled by kstrtoint.

Signed-off-by: LABBE Corentin <[email protected]>
---
drivers/atm/solos-pci.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 3d7fb65..2bec4f1 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -347,8 +347,8 @@ static char *next_string(struct sk_buff *skb)
*/
static int process_status(struct solos_card *card, int port, struct sk_buff *skb)
{
- char *str, *end, *state_str, *snr, *attn;
- int ver, rate_up, rate_down;
+ char *str, *state_str, *snr, *attn;
+ int ver, rate_up, rate_down, err;

if (!card->atmdev[port])
return -ENODEV;
@@ -357,11 +357,11 @@ static int process_status(struct solos_card *card, int port, struct sk_buff *skb
if (!str)
return -EIO;

- ver = simple_strtol(str, NULL, 10);
- if (ver < 1) {
+ err = kstrtoint(str, 10, &ver);
+ if (err || ver < 1) {
dev_warn(&card->dev->dev, "Unexpected status interrupt version %d\n",
ver);
- return -EIO;
+ return err;
}

str = next_string(skb);
@@ -373,16 +373,16 @@ static int process_status(struct solos_card *card, int port, struct sk_buff *skb
return 0;
}

- rate_down = simple_strtol(str, &end, 10);
- if (*end)
- return -EIO;
+ err = kstrtoint(str, 10, &rate_down);
+ if (err)
+ return err;

str = next_string(skb);
if (!str)
return -EIO;
- rate_up = simple_strtol(str, &end, 10);
- if (*end)
- return -EIO;
+ err = kstrtoint(str, 10, &rate_up);
+ if (err)
+ return err;

state_str = next_string(skb);
if (!state_str)
@@ -417,7 +417,7 @@ static int process_command(struct solos_card *card, int port, struct sk_buff *sk
struct solos_param *prm;
unsigned long flags;
int cmdpid;
- int found = 0;
+ int found = 0, err;

if (skb->len < 7)
return 0;
@@ -428,7 +428,9 @@ static int process_command(struct solos_card *card, int port, struct sk_buff *sk
skb->data[6] != '\n')
return 0;

- cmdpid = simple_strtol(&skb->data[1], NULL, 10);
+ err = kstrtoint(&skb->data[1], 10, &cmdpid);
+ if (err)
+ return err;

spin_lock_irqsave(&card->param_queue_lock, flags);
list_for_each_entry(prm, &card->param_queue, list) {
--
2.4.10

2015-12-03 11:26:34

by Chas Williams

Subject: Re: [PATCH v3 1/1] atm: solos-pci: Replace simple_strtol by kstrtoint

On Thu, 2015-12-03 at 09:06 +0100, LABBE Corentin wrote:
> @@ -357,11 +357,11 @@ static int process_status(struct solos_card *card, int port, struct sk_buff *skb
> if (!str)
> return -EIO;
>
> - ver = simple_strtol(str, NULL, 10);
> - if (ver < 1) {
> + err = kstrtoint(str, 10, &ver);
> + if (err || ver < 1) {
> dev_warn(&card->dev->dev, "Unexpected status interrupt version %d\n",
> ver);
> - return -EIO;
> + return err;


If ver < 1 then you might return a 0 here. Always returning -EIO is
probably just fine.

2015-12-03 12:58:51

by Corentin Labbe

Subject: Re: [PATCH v3 1/1] atm: solos-pci: Replace simple_strtol by kstrtoint

On Thu, Dec 03, 2015 at 06:26:31AM -0500, Charles (Chas) Williams wrote:
> On Thu, 2015-12-03 at 09:06 +0100, LABBE Corentin wrote:
> > @@ -357,11 +357,11 @@ static int process_status(struct solos_card *card, int port, struct sk_buff *skb
> > if (!str)
> > return -EIO;
> >
> > - ver = simple_strtol(str, NULL, 10);
> > - if (ver < 1) {
> > + err = kstrtoint(str, 10, &ver);
> > + if (err || ver < 1) {
> > dev_warn(&card->dev->dev, "Unexpected status interrupt version %d\n",
> > ver);
> > - return -EIO;
> > + return err;
>
>
> If ver < 1 then you might return a 0 here. Always returning -EIO is
> probably just fine.
>

Hello

I think the best solution is to split the test, since returning error code from kstrtoint was asked by David Miller.
if (err)
return err;
if (ver < 1)
return -EIO;
Thanks
Regards

2015-12-03 16:34:10

by Chas Williams

Subject: Re: [PATCH v3 1/1] atm: solos-pci: Replace simple_strtol by kstrtoint

On Thu, 2015-12-03 at 13:58 +0100, LABBE Corentin wrote:
> On Thu, Dec 03, 2015 at 06:26:31AM -0500, Charles (Chas) Williams wrote:
> > On Thu, 2015-12-03 at 09:06 +0100, LABBE Corentin wrote:
> > > @@ -357,11 +357,11 @@ static int process_status(struct solos_card *card, int port, struct sk_buff *skb
> > > if (!str)
> > > return -EIO;
> > >
> > > - ver = simple_strtol(str, NULL, 10);
> > > - if (ver < 1) {
> > > + err = kstrtoint(str, 10, &ver);
> > > + if (err || ver < 1) {
> > > dev_warn(&card->dev->dev, "Unexpected status interrupt version %d\n",
> > > ver);
> > > - return -EIO;
> > > + return err;
> >
> >
> > If ver < 1 then you might return a 0 here. Always returning -EIO is
> > probably just fine.
> >
>
> Hello
>
> I think the best solution is to split the test, since returning error code from kstrtoint was asked by David Miller.
> if (err)
> return err;
> if (ver < 1)
> return -EIO;
> Thanks
> Regards

That's fine. You just shouldn't return 0 if the ver < 1. This isn't
timing critical code.