* [PATCH] dspbridge: Simplify Atoi() method
@ 2010-02-03 10:14 Andy Shevchenko
2010-02-10 2:02 ` Omar Ramirez Luna
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2010-02-03 10:14 UTC (permalink / raw)
To: linux-omap; +Cc: Andy Shevchenko
From: Andy Shevchenko <ext-andriy.shevchenko@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.shevchenko@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] dspbridge: Simplify Atoi() method
2010-02-03 10:14 [PATCH] dspbridge: Simplify Atoi() method Andy Shevchenko
@ 2010-02-10 2:02 ` Omar Ramirez Luna
2010-02-10 7:58 ` [PATCH] dspbridge: Simplify Atoi() method (v2) Andy Shevchenko
2010-02-10 7:59 ` [PATCH] dspbridge: Simplify Atoi() method Andy Shevchenko
0 siblings, 2 replies; 6+ messages in thread
From: Omar Ramirez Luna @ 2010-02-10 2:02 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-omap, Andy Shevchenko
Hi,
On 2/3/2010 4:14 AM, Andy Shevchenko wrote:
> From: Andy Shevchenko<ext-andriy.shevchenko@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 Shevchenko<ext-andriy.shevchenko@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] dspbridge: Simplify Atoi() method (v2)
2010-02-10 2:02 ` Omar Ramirez Luna
@ 2010-02-10 7:58 ` Andy Shevchenko
2010-02-16 23:49 ` Omar Ramirez Luna
2010-02-10 7:59 ` [PATCH] dspbridge: Simplify Atoi() method Andy Shevchenko
1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2010-02-10 7:58 UTC (permalink / raw)
To: linux-omap; +Cc: Andy Shevchenko
From: Andy Shevchenko <ext-andriy.shevchenko@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.shevchenko@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] dspbridge: Simplify Atoi() method
2010-02-10 2:02 ` Omar Ramirez Luna
2010-02-10 7:58 ` [PATCH] dspbridge: Simplify Atoi() method (v2) Andy Shevchenko
@ 2010-02-10 7:59 ` Andy Shevchenko
2010-02-10 15:55 ` Omar Ramirez Luna
1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2010-02-10 7:59 UTC (permalink / raw)
To: Omar Ramirez Luna; +Cc: linux-omap
On Wed, Feb 10, 2010 at 4:02 AM, Omar Ramirez Luna <omar.ramirez@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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dspbridge: Simplify Atoi() method
2010-02-10 7:59 ` [PATCH] dspbridge: Simplify Atoi() method Andy Shevchenko
@ 2010-02-10 15:55 ` Omar Ramirez Luna
0 siblings, 0 replies; 6+ messages in thread
From: Omar Ramirez Luna @ 2010-02-10 15:55 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-omap
On 2/10/2010 1:59 AM, Andy Shevchenko wrote:
> On Wed, Feb 10, 2010 at 4:02 AM, Omar Ramirez Luna<omar.ramirez@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dspbridge: Simplify Atoi() method (v2)
2010-02-10 7:58 ` [PATCH] dspbridge: Simplify Atoi() method (v2) Andy Shevchenko
@ 2010-02-16 23:49 ` Omar Ramirez Luna
0 siblings, 0 replies; 6+ messages in thread
From: Omar Ramirez Luna @ 2010-02-16 23:49 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-omap, Andy Shevchenko
On 2/10/2010 1:58 AM, Andy Shevchenko wrote:
> From: Andy Shevchenko<ext-andriy.shevchenko@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.shevchenko@nokia.com>
Acked-by: Omar Ramirez Luna <omar.ramirez@ti.com>
Pushed to dspbridge
- omar
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-02-16 23:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-03 10:14 [PATCH] dspbridge: Simplify Atoi() method Andy Shevchenko
2010-02-10 2:02 ` Omar Ramirez Luna
2010-02-10 7:58 ` [PATCH] dspbridge: Simplify Atoi() method (v2) Andy Shevchenko
2010-02-16 23:49 ` Omar Ramirez Luna
2010-02-10 7:59 ` [PATCH] dspbridge: Simplify Atoi() method Andy Shevchenko
2010-02-10 15:55 ` Omar Ramirez Luna
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox