[PATCH v2] lib: Improve _parse_integer_fixup_radix base 16 detection
Base autodetection is failing for this case: if test 257 -gt 3ae; then echo first; else echo second; fi
It is because base for 3ae is recognized by _parse_integer_fixup_radix() as 10. The patch is checking all chars to make sure that they are not 'a' or up. If they are base needs to be in hex.
Signed-off-by: Michal Simek michal.simek@xilinx.com Signed-off-by: Shiril Tichkule shirilt@xlinx.com Reviewed-by: Tom Rini trini@konsulko.com ---
Changes in v2: - Fix end of string to be \0 instead of \n - Detect hex by checking chars between a-f to avoid cases like number follow by ;
lib/strto.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/lib/strto.c b/lib/strto.c index 55ff9f7437d5..4ac79f46a5e2 100644 --- a/lib/strto.c +++ b/lib/strto.c @@ -22,9 +22,26 @@ static const char *_parse_integer_fixup_radix(const char *s, unsigned int *base) *base = 16; else *base = 8; - } else + } else { + int i; + *base = 10; + + for (i = 0; ; i++) { + char var = s[i]; + + if (var == '\0') + break; + + if ((var >= 'a' && var <= 'f') || + (var >= 'A' && var <= 'F')) { + *base = 16; + break; + } + } + } } + if (*base == 16 && s[0] == '0' && tolower(s[1]) == 'x') s += 2; return s;
On Mon, Mar 2, 2020 at 10:36 AM Michal Simek michal.simek@xilinx.com wrote:
Base autodetection is failing for this case: if test 257 -gt 3ae; then echo first; else echo second; fi
It is because base for 3ae is recognized by _parse_integer_fixup_radix() as 10. The patch is checking all chars to make sure that they are not 'a' or up. If they are base needs to be in hex.
...
for (i = 0; ; i++) {char var = s[i];if (var == '\0')break;if ((var >= 'a' && var <= 'f') ||(var >= 'A' && var <= 'F')) {*base = 16;break;}}
int i = 0; char var;
do { var = tolower(s[i++]); if (...) { *base = 16; break; } } while (var);
or alike?
...
}
Is this relevant?
if (*base == 16 && s[0] == '0' && tolower(s[1]) == 'x') s += 2; return s;
On 03. 03. 20 10:37, Andy Shevchenko wrote:
On Mon, Mar 2, 2020 at 10:36 AM Michal Simek michal.simek@xilinx.com wrote:
Base autodetection is failing for this case: if test 257 -gt 3ae; then echo first; else echo second; fi
It is because base for 3ae is recognized by _parse_integer_fixup_radix() as 10. The patch is checking all chars to make sure that they are not 'a' or up. If they are base needs to be in hex.
...
for (i = 0; ; i++) {char var = s[i];if (var == '\0')break;if ((var >= 'a' && var <= 'f') ||(var >= 'A' && var <= 'F')) {*base = 16;break;}}int i = 0; char var;
do { var = tolower(s[i++]); if (...) { *base = 16; break; } } while (var);
or alike?
not a problem with this.
...
}
Is this relevant?
it can be removed from this patch but I can't see any issue with it to making code more readable.
if (*base == 16 && s[0] == '0' && tolower(s[1]) == 'x') s += 2; return s;
M
On Tue, Mar 3, 2020 at 2:03 PM Michal Simek michal.simek@xilinx.com wrote:
On 03. 03. 20 10:37, Andy Shevchenko wrote:
On Mon, Mar 2, 2020 at 10:36 AM Michal Simek michal.simek@xilinx.com wrote:
Base autodetection is failing for this case: if test 257 -gt 3ae; then echo first; else echo second; fi
It is because base for 3ae is recognized by _parse_integer_fixup_radix() as 10.
The patch is checking all chars to make sure that they are not 'a' or up. If they are base needs to be in hex.
Hold on!
for (i = 0; ; i++) {char var = s[i];if (var == '\0')break;if ((var >= 'a' && var <= 'f') ||(var >= 'A' && var <= 'F')) {*base = 16;break;}}
Actually your code and commit message are unaligned. In the code you stop on the *first* hex character without checking the rest.
On 03. 03. 20 15:52, Andy Shevchenko wrote:
On Tue, Mar 3, 2020 at 2:03 PM Michal Simek michal.simek@xilinx.com wrote:
On 03. 03. 20 10:37, Andy Shevchenko wrote:
On Mon, Mar 2, 2020 at 10:36 AM Michal Simek michal.simek@xilinx.com wrote:
Base autodetection is failing for this case: if test 257 -gt 3ae; then echo first; else echo second; fi
It is because base for 3ae is recognized by _parse_integer_fixup_radix() as 10.
The patch is checking all chars to make sure that they are not 'a' or up. If they are base needs to be in hex.
Hold on!
for (i = 0; ; i++) {char var = s[i];if (var == '\0')break;if ((var >= 'a' && var <= 'f') ||(var >= 'A' && var <= 'F')) {*base = 16;break;}}Actually your code and commit message are unaligned. In the code you stop on the *first* hex character without checking the rest.
That's correct because base is detected as 10 at this stage.
The question is if make sense to check all chars because if there is one char between a<= <=f then you are sure that it is not base 10.
And if you want to check all of them then you need to also detect chars like ';' which can be there as you see above.
Thanks, Michal
participants (2)
-
Andy Shevchenko -
Michal Simek