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 <t.karthik.re...@xilinx.com>
>>>>>>
>>>>>> This patch uses auto instead of decimal in simple_strtoul().
>>>>>>
>>>>>> Signed-off-by: T Karthik Reddy <t.karthik.re...@xilinx.com>
>>>>>> Signed-off-by: Michal Simek <michal.si...@xilinx.com>
>>>>>> ---
>>>>>>
>>>>>> 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.
Thanks,
Michal
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
