On Thu, Oct 10, 2019 at 12:46 PM Michal Simek <[email protected]> wrote: > > On 09. 10. 19 19:28, Simon Goldschmidt wrote: > > Am 09.10.2019 um 18:26 schrieb Tom Rini: > >> On Tue, Oct 08, 2019 at 10:48:39AM +0200, Michal Simek wrote: > >>> Hi Tom, > >>> > >>> On 19. 09. 19 15:28, Michal Simek wrote: > >>>> On 13. 09. 19 17:09, Tom Rini wrote: > >>>>> On Wed, Sep 11, 2019 at 03:39:53PM +0200, Michal Simek wrote: > >>>>> > >>>>>> From: T Karthik Reddy <[email protected]> > >>>>>> > >>>>>> This patch uses auto instead of decimal in simple_strtoul(). > >>>>>> > >>>>>> Signed-off-by: T Karthik Reddy <[email protected]> > >>>>>> Signed-off-by: Michal Simek <[email protected]> > >>>>>> --- > >>>>>> > >>>>>> cmd/test.c | 24 ++++++++++++------------ > >>>>>> 1 file changed, 12 insertions(+), 12 deletions(-) > >>>>>> > >>>>>> diff --git a/cmd/test.c b/cmd/test.c > >>>>>> index fa0c349f0827..258bfd880653 100644 > >>>>>> --- a/cmd/test.c > >>>>>> +++ b/cmd/test.c > >>>>>> @@ -113,28 +113,28 @@ static int do_test(cmd_tbl_t *cmdtp, int > >>>>>> flag, int argc, char * const argv[]) > >>>>>> expr = strcmp(ap[0], ap[2]) > 0; > >>>>>> break; > >>>>>> case OP_INT_EQ: > >>>>>> - expr = simple_strtol(ap[0], NULL, 10) == > >>>>>> - simple_strtol(ap[2], NULL, 10); > >>>>>> + expr = simple_strtol(ap[0], NULL, 0) == > >>>>>> + simple_strtol(ap[2], NULL, 0); > >>>>>> break; > >>>>>> case OP_INT_NEQ: > >>>>>> - expr = simple_strtol(ap[0], NULL, 10) != > >>>>>> - simple_strtol(ap[2], NULL, 10); > >>>>>> + expr = simple_strtol(ap[0], NULL, 0) != > >>>>>> + simple_strtol(ap[2], NULL, 0); > >>>>>> break; > >>>>>> case OP_INT_LT: > >>>>>> - expr = simple_strtol(ap[0], NULL, 10) < > >>>>>> - simple_strtol(ap[2], NULL, 10); > >>>>>> + expr = simple_strtol(ap[0], NULL, 0) < > >>>>>> + simple_strtol(ap[2], NULL, 0); > >>>>>> break; > >>>>>> case OP_INT_LE: > >>>>>> - expr = simple_strtol(ap[0], NULL, 10) <= > >>>>>> - simple_strtol(ap[2], NULL, 10); > >>>>>> + expr = simple_strtol(ap[0], NULL, 0) <= > >>>>>> + simple_strtol(ap[2], NULL, 0); > >>>>>> break; > >>>>>> case OP_INT_GT: > >>>>>> - expr = simple_strtol(ap[0], NULL, 10) > > >>>>>> - simple_strtol(ap[2], NULL, 10); > >>>>>> + expr = simple_strtol(ap[0], NULL, 0) > > >>>>>> + simple_strtol(ap[2], NULL, 0); > >>>>>> break; > >>>>>> case OP_INT_GE: > >>>>>> - expr = simple_strtol(ap[0], NULL, 10) >= > >>>>>> - simple_strtol(ap[2], NULL, 10); > >>>>>> + expr = simple_strtol(ap[0], NULL, 0) >= > >>>>>> + simple_strtol(ap[2], NULL, 0); > >>>>>> break; > >>>>>> case OP_FILE_EXISTS: > >>>>>> expr = file_exists(ap[1], ap[2], ap[3], FS_TYPE_ANY); > >>>>> > >>>>> I'm going to NAK this, but could be argued around to changing my mind. > >>>>> While it's true that in general command inputs are hex and not > >>>>> decimal, > >>>>> this has been decimal since introduction in 2009. So changing it > >>>>> now is > >>>>> breaking ABI and other peoples test scripts, so I don't think we > >>>>> can do > >>>>> this, sorry. > >>>> > >>>> I also think that this is not breaking any ABI. test_hush_if_test.py is > >>>> around for a while to capture issues in this space and I can't see any > >>>> single failure in connection to this change. > >>>> > >>>> If this accepted then we can add more tests like this > >>>> ('test 0x2000000 -gt 0x2000001', False), > >>>> ('test 0x2000000 -gt 0x2000000', False), > >>>> ('test 0x2000000 -gt 0x1ffffff', True), > >>>> ('test 2000000 -gt 0x1ffffff', False), > >>>> ('test 0x2000000 -gt 1ffffff', True), > >>>> > >>>> ('test 0x2000000 -lt 1ffffff', False), > >>>> ('test 0x2000000 -eq 2000000', False), > >>>> ('test 0x2000000 -ne 2000000', True), > >>>> > >>>> where some of them are failing without this patch > >>>> ... test_hush_if_test[test 0x2000000 -gt 0x1ffffff-True] > >>>> > >>>> ... test_hush_if_test[test 2000000 -gt 0x1ffffff-False] > >>>> > >>>> ... test_hush_if_test[test 0x2000000 -gt 1ffffff-True] > >>>> > >>>> ... test_hush_if_test[test 0x2000000 -lt 1ffffff-False] > >>>> > >>> > >>> Any comment on this? > >> > >> Sorry, yes, OK, we can take this then. I should have this in my first > >> batch of non-python general changes I grab. > > > > But strtoul("010", NULL, 0) is 8 while strtoul("010", NULL, 10) is 10. > > Do we care for that change? > > Leading zeroes are problematic > > Before the patch this pass > test 010 -eq 10 && echo yes > > After the patch this fail. > > I can't see any test in test/py/tests/test_hush_if_test.py to cover this > case that's why I can't guess if this is used or not.
Hmm, it might not be a problem then...? Regards, Simon _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
