From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Goldschmidt Date: Wed, 9 Oct 2019 19:28:50 +0200 Subject: [U-Boot] [PATCH] cmd: avoid decimal conversion In-Reply-To: <20191009162651.GP6716@bill-the-cat> References: <068609d5f2936454d3a999de99317f1def78ceea.1568209191.git.michal.simek@xilinx.com> <20190913150905.GA5945@hodge-podge> <20191009162651.GP6716@bill-the-cat> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>>>> >>>>> This patch uses auto instead of decimal in simple_strtoul(). >>>>> >>>>> Signed-off-by: T Karthik Reddy >>>>> Signed-off-by: Michal Simek >>>>> --- >>>>> >>>>> 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