* [U-Boot] [PATCH] cmd: avoid decimal conversion
@ 2019-09-11 13:39 Michal Simek
2019-09-13 15:09 ` Tom Rini
2019-10-12 20:23 ` Tom Rini
0 siblings, 2 replies; 10+ messages in thread
From: Michal Simek @ 2019-09-11 13:39 UTC (permalink / raw)
To: u-boot
From: T Karthik Reddy <t.karthik.reddy@xilinx.com>
This patch uses auto instead of decimal in simple_strtoul().
Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@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);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] cmd: avoid decimal conversion
2019-09-11 13:39 [U-Boot] [PATCH] cmd: avoid decimal conversion Michal Simek
@ 2019-09-13 15:09 ` Tom Rini
2019-09-13 15:17 ` Michael Nazzareno Trimarchi
2019-09-19 13:28 ` Michal Simek
2019-10-12 20:23 ` Tom Rini
1 sibling, 2 replies; 10+ messages in thread
From: Tom Rini @ 2019-09-13 15:09 UTC (permalink / raw)
To: u-boot
On Wed, Sep 11, 2019 at 03:39:53PM +0200, Michal Simek wrote:
> From: T Karthik Reddy <t.karthik.reddy@xilinx.com>
>
> This patch uses auto instead of decimal in simple_strtoul().
>
> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190913/da42964f/attachment.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] cmd: avoid decimal conversion
2019-09-13 15:09 ` Tom Rini
@ 2019-09-13 15:17 ` Michael Nazzareno Trimarchi
2019-09-19 13:28 ` Michal Simek
1 sibling, 0 replies; 10+ messages in thread
From: Michael Nazzareno Trimarchi @ 2019-09-13 15:17 UTC (permalink / raw)
To: u-boot
On Fri, Sep 13, 2019 at 5:10 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Sep 11, 2019 at 03:39:53PM +0200, Michal Simek wrote:
>
> > From: T Karthik Reddy <t.karthik.reddy@xilinx.com>
> >
> > This patch uses auto instead of decimal in simple_strtoul().
> >
> > Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
> > Signed-off-by: Michal Simek <michal.simek@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.
Well, I think that this is automatic. There should be a back compatibility
with the ABI if base is 10
Michael
>
> --
> Tom
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
--
| Michael Nazzareno Trimarchi Amarula Solutions BV |
| COO - Founder Cruquiuskade 47 |
| +31(0)851119172 Amsterdam 1018 AM NL |
| [`as] http://www.amarulasolutions.com |
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] cmd: avoid decimal conversion
2019-09-13 15:09 ` Tom Rini
2019-09-13 15:17 ` Michael Nazzareno Trimarchi
@ 2019-09-19 13:28 ` Michal Simek
2019-10-08 8:48 ` Michal Simek
1 sibling, 1 reply; 10+ messages in thread
From: Michal Simek @ 2019-09-19 13:28 UTC (permalink / raw)
To: u-boot
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@xilinx.com>
>>
>> This patch uses auto instead of decimal in simple_strtoul().
>>
>> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
>> Signed-off-by: Michal Simek <michal.simek@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]
Thanks,
Michal
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] cmd: avoid decimal conversion
2019-09-19 13:28 ` Michal Simek
@ 2019-10-08 8:48 ` Michal Simek
2019-10-09 16:26 ` Tom Rini
0 siblings, 1 reply; 10+ messages in thread
From: Michal Simek @ 2019-10-08 8:48 UTC (permalink / raw)
To: u-boot
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@xilinx.com>
>>>
>>> This patch uses auto instead of decimal in simple_strtoul().
>>>
>>> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
>>> Signed-off-by: Michal Simek <michal.simek@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] cmd: avoid decimal conversion
2019-10-08 8:48 ` Michal Simek
@ 2019-10-09 16:26 ` Tom Rini
2019-10-09 17:28 ` Simon Goldschmidt
0 siblings, 1 reply; 10+ messages in thread
From: Tom Rini @ 2019-10-09 16:26 UTC (permalink / raw)
To: u-boot
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.reddy@xilinx.com>
> >>>
> >>> This patch uses auto instead of decimal in simple_strtoul().
> >>>
> >>> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
> >>> Signed-off-by: Michal Simek <michal.simek@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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191009/81ec862b/attachment-0001.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] cmd: avoid decimal conversion
2019-10-09 16:26 ` Tom Rini
@ 2019-10-09 17:28 ` Simon Goldschmidt
2019-10-10 10:46 ` Michal Simek
0 siblings, 1 reply; 10+ messages in thread
From: Simon Goldschmidt @ 2019-10-09 17:28 UTC (permalink / raw)
To: u-boot
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.reddy@xilinx.com>
>>>>>
>>>>> This patch uses auto instead of decimal in simple_strtoul().
>>>>>
>>>>> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
>>>>> Signed-off-by: Michal Simek <michal.simek@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?
Regards,
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] cmd: avoid decimal conversion
2019-10-09 17:28 ` Simon Goldschmidt
@ 2019-10-10 10:46 ` Michal Simek
2019-10-10 10:49 ` Simon Goldschmidt
0 siblings, 1 reply; 10+ messages in thread
From: Michal Simek @ 2019-10-10 10:46 UTC (permalink / raw)
To: u-boot
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.reddy@xilinx.com>
>>>>>>
>>>>>> This patch uses auto instead of decimal in simple_strtoul().
>>>>>>
>>>>>> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
>>>>>> Signed-off-by: Michal Simek <michal.simek@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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] cmd: avoid decimal conversion
2019-10-10 10:46 ` Michal Simek
@ 2019-10-10 10:49 ` Simon Goldschmidt
0 siblings, 0 replies; 10+ messages in thread
From: Simon Goldschmidt @ 2019-10-10 10:49 UTC (permalink / raw)
To: u-boot
On Thu, Oct 10, 2019 at 12:46 PM Michal Simek <michal.simek@xilinx.com> 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 <t.karthik.reddy@xilinx.com>
> >>>>>>
> >>>>>> This patch uses auto instead of decimal in simple_strtoul().
> >>>>>>
> >>>>>> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
> >>>>>> Signed-off-by: Michal Simek <michal.simek@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.
Hmm, it might not be a problem then...?
Regards,
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH] cmd: avoid decimal conversion
2019-09-11 13:39 [U-Boot] [PATCH] cmd: avoid decimal conversion Michal Simek
2019-09-13 15:09 ` Tom Rini
@ 2019-10-12 20:23 ` Tom Rini
1 sibling, 0 replies; 10+ messages in thread
From: Tom Rini @ 2019-10-12 20:23 UTC (permalink / raw)
To: u-boot
On Wed, Sep 11, 2019 at 03:39:53PM +0200, Michal Simek wrote:
> From: T Karthik Reddy <t.karthik.reddy@xilinx.com>
>
> This patch uses auto instead of decimal in simple_strtoul().
>
> Signed-off-by: T Karthik Reddy <t.karthik.reddy@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20191012/553888ff/attachment.sig>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-10-12 20:23 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-11 13:39 [U-Boot] [PATCH] cmd: avoid decimal conversion Michal Simek
2019-09-13 15:09 ` Tom Rini
2019-09-13 15:17 ` Michael Nazzareno Trimarchi
2019-09-19 13:28 ` Michal Simek
2019-10-08 8:48 ` Michal Simek
2019-10-09 16:26 ` Tom Rini
2019-10-09 17:28 ` Simon Goldschmidt
2019-10-10 10:46 ` Michal Simek
2019-10-10 10:49 ` Simon Goldschmidt
2019-10-12 20:23 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.