[U-Boot] [PATCH] cmd: avoid decimal conversion
Michal Simek
michal.simek at xilinx.com
Tue Oct 8 08:48:39 UTC 2019
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.reddy at xilinx.com>
>>>
>>> This patch uses auto instead of decimal in simple_strtoul().
>>>
>>> Signed-off-by: T Karthik Reddy <t.karthik.reddy at xilinx.com>
>>> Signed-off-by: Michal Simek <michal.simek at 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?
Thanks,
Michal
More information about the U-Boot
mailing list