Re: [PATCH] dspbridge: Simplify Atoi() method (v2)

[Omar Ramirez Luna]

On 2/10/2010 1:58 AM, Andy Shevchenko wrote:

From: Andy Shevchenkoext-andriy.shevche...@nokia.com

Try to use simple_strtoul() kernel native method instead.

However, there are opened questions:
  - why type of Atoi() is s32 if the sign is used only to detect base?
  - should we really to check hex integers like DEAD0123h?
  - how many spaces could lead token?

Signed-off-by: Andy Shevchenkoext-andriy.shevche...@nokia.com


Acked-by: Omar Ramirez Luna omar.rami...@ti.com

Pushed to dspbridge

- omar
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] dspbridge: Simplify Atoi() method (v2)

[Andy Shevchenko]

From: Andy Shevchenko ext-andriy.shevche...@nokia.com

Try to use simple_strtoul() kernel native method instead.

However, there are opened questions:
 - why type of Atoi() is s32 if the sign is used only to detect base?
 - should we really to check hex integers like DEAD0123h?
 - how many spaces could lead token?

Signed-off-by: Andy Shevchenko ext-andriy.shevche...@nokia.com
---
 drivers/dsp/bridge/rmgr/dbdcd.c |   42 +-
 1 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/drivers/dsp/bridge/rmgr/dbdcd.c b/drivers/dsp/bridge/rmgr/dbdcd.c
index 9efb7dc..a460d1a 100644
--- a/drivers/dsp/bridge/rmgr/dbdcd.c
+++ b/drivers/dsp/bridge/rmgr/dbdcd.c
@@ -1001,50 +1001,20 @@ DSP_STATUS DCD_UnregisterObject(IN struct DSP_UUID 
*pUuid,
  */
 static s32 Atoi(char *pszBuf)
 {
-   s32 result = 0;
char *pch = pszBuf;
-   char c;
-   char first;
-   s32 base = 10;
-   s32 len;
+   s32 base = 0;
 
while (isspace(*pch))
pch++;
 
-   first = *pch;
-   if (first == '-' || first == '+') {
+   if (*pch == '-' || *pch == '+') {
+   base = 10;
pch++;
-   } else {
-   /* Determine if base 10 or base 16 */
-   len = strlen(pch);
-   if (len   1) {
-   c = pch[1];
-   if ((*pch == '0'  (c == 'x' || c == 'X'))) {
-   base = 16;
-   pch += 2;
-   }
-   c = pch[len - 1];
-   if (c == 'h' || c == 'H')
-   base = 16;
-
-   }
-   }
-
-   while (isdigit(c = *pch) || ((base == 16)  isxdigit(c))) {
-   result *= base;
-   if ('A' = c  c = 'F') {
-   c = c - 'A' + 10;
-   } else {
-   if ('a' = c  c = 'f')
-   c = c - 'a' + 10;
-   else
-   c -= '0';
-   }
-   result += c;
-   ++pch;
+   } else if (*pch  tolower(pch[strlen(pch) - 1]) == 'h') {
+   base = 16;
}
 
-   return result;
+   return simple_strtoul(pch, NULL, base);
 }
 
 /*
-- 
1.5.6.5

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dspbridge: Simplify Atoi() method

[Omar Ramirez Luna]

On 2/10/2010 1:59 AM, Andy Shevchenko wrote:

On Wed, Feb 10, 2010 at 4:02 AM, Omar Ramirez Lunaomar.rami...@ti.com  wrote:

Try to use simple_strtol() kernel native method instead.

strtol or strtoul?

I don't know for sure, see below.


sorry, i meant: here it says simple_strtol but the code says simple_strtoul




However, there are opened questions:
  - why type of Atoi() is s32 if the sign is used only to detect base?

This is the question about l vs ul.


agree, IMO this should be ul, I haven't reviewed all the places where 
Atoi is used but the ones I have seen use it as ul


about the sign: I haven't seen any value with '+' or '-' prefix so with 
this patch the decision to identify as base10 is left to 
simple_guess_base inside strtoul


if value strings had '0x' instead of ending 'H' for hex the whole Atoi 
could be replaced, i guess this would also allow to use strict_strtoul 
and avoid a checkpatch warning :(


- omar

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dspbridge: Simplify Atoi() method

[Omar Ramirez Luna]

Hi,

On 2/3/2010 4:14 AM, Andy Shevchenko wrote:

From: Andy Shevchenkoext-andriy.shevche...@nokia.com

Try to use simple_strtol() kernel native method instead.


strtol or strtoul?



However, there are opened questions:
  - why type of Atoi() is s32 if the sign is used only to detect base?
  - should we really to check hex integers like DEAD0123h?
  - how many spaces could lead token?

Signed-off-by: Andy Shevchenkoext-andriy.shevche...@nokia.com
---
  drivers/dsp/bridge/rmgr/dbdcd.c |   42 +-
  1 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/drivers/dsp/bridge/rmgr/dbdcd.c b/drivers/dsp/bridge/rmgr/dbdcd.c
index caa57f1..fe2ed57 100644
--- a/drivers/dsp/bridge/rmgr/dbdcd.c
+++ b/drivers/dsp/bridge/rmgr/dbdcd.c
@@ -1002,50 +1002,20 @@ DSP_STATUS DCD_UnregisterObject(IN struct DSP_UUID 
*pUuid,
   */
  static s32 Atoi(char *pszBuf)
  {
-   s32 result = 0;
char *pch = pszBuf;
-   char c;
-   char first;
-   s32 base = 10;
-   s32 len;
+   s32 base = 0;

while (isspace(*pch))
pch++;

-   first = *pch;
-   if (first == '-' || first == '+') {
+   if (*pch == '-' || *pch == '+') {
+   base = 10;
pch++;
-   } else {
-   /* Determine if base 10 or base 16 */
-   len = strlen(pch);
-   if (len  1) {
-   c = pch[1];
-   if ((*pch == '0'  (c == 'x' || c == 'X'))) {
-   base = 16;
-   pch += 2;
-   }
-   c = pch[len - 1];
-   if (c == 'h' || c == 'H')
-   base = 16;
-
-   }
-   }
-
-   while (isdigit(c = *pch) || ((base == 16)  isxdigit(c))) {
-   result *= base;
-   if ('A'= c  c= 'F') {
-   c = c - 'A' + 10;
-   } else {
-   if ('a'= c  c= 'f')
-   c = c - 'a' + 10;
-   else
-   c -= '0';
-   }
-   result += c;
-   ++pch;
+   } else if (*pch  (pch[strlen(pch) - 1] | 0x20 == 'h')) {


perhaps tolower(x)

otherwise: (pch[strlen(pch) - 1] | 0x20 == 'h')
should be ((pch[strlen(pch) - 1] | 0x20) == 'h')


+   base = 16;
}

-   return result;
+   return simple_strtoul(pch, NULL, base);
  }

  /*


- omar
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dspbridge: Simplify Atoi() method

[Andy Shevchenko]

On Wed, Feb 10, 2010 at 4:02 AM, Omar Ramirez Luna omar.rami...@ti.com wrote:
 Try to use simple_strtol() kernel native method instead.
 strtol or strtoul?
I don't know for sure, see below.

 However, there are opened questions:
  - why type of Atoi() is s32 if the sign is used only to detect base?
This is the question about l vs ul.

 +       } else if (*pch  (pch[strlen(pch) - 1] | 0x20 == 'h')) {

 perhaps tolower(x)
Yep, I saw only internal macro in lib/vsprintf.c, but totally forgot
about ctype.h.

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] dspbridge: Simplify Atoi() method

[Andy Shevchenko]

From: Andy Shevchenko ext-andriy.shevche...@nokia.com

Try to use simple_strtol() kernel native method instead.

However, there are opened questions:
 - why type of Atoi() is s32 if the sign is used only to detect base?
 - should we really to check hex integers like DEAD0123h?
 - how many spaces could lead token?

Signed-off-by: Andy Shevchenko ext-andriy.shevche...@nokia.com
---
 drivers/dsp/bridge/rmgr/dbdcd.c |   42 +-
 1 files changed, 6 insertions(+), 36 deletions(-)

diff --git a/drivers/dsp/bridge/rmgr/dbdcd.c b/drivers/dsp/bridge/rmgr/dbdcd.c
index caa57f1..fe2ed57 100644
--- a/drivers/dsp/bridge/rmgr/dbdcd.c
+++ b/drivers/dsp/bridge/rmgr/dbdcd.c
@@ -1002,50 +1002,20 @@ DSP_STATUS DCD_UnregisterObject(IN struct DSP_UUID 
*pUuid,
  */
 static s32 Atoi(char *pszBuf)
 {
-   s32 result = 0;
char *pch = pszBuf;
-   char c;
-   char first;
-   s32 base = 10;
-   s32 len;
+   s32 base = 0;
 
while (isspace(*pch))
pch++;
 
-   first = *pch;
-   if (first == '-' || first == '+') {
+   if (*pch == '-' || *pch == '+') {
+   base = 10;
pch++;
-   } else {
-   /* Determine if base 10 or base 16 */
-   len = strlen(pch);
-   if (len   1) {
-   c = pch[1];
-   if ((*pch == '0'  (c == 'x' || c == 'X'))) {
-   base = 16;
-   pch += 2;
-   }
-   c = pch[len - 1];
-   if (c == 'h' || c == 'H')
-   base = 16;
-
-   }
-   }
-
-   while (isdigit(c = *pch) || ((base == 16)  isxdigit(c))) {
-   result *= base;
-   if ('A' = c  c = 'F') {
-   c = c - 'A' + 10;
-   } else {
-   if ('a' = c  c = 'f')
-   c = c - 'a' + 10;
-   else
-   c -= '0';
-   }
-   result += c;
-   ++pch;
+   } else if (*pch  (pch[strlen(pch) - 1] | 0x20 == 'h')) {
+   base = 16;
}
 
-   return result;
+   return simple_strtoul(pch, NULL, base);
 }
 
 /*
-- 
1.5.6.5

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html