| Message ID | 20200311224652.16900-1-christoph.muellner@theobroma-systems.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Michal Simek |
| Headers | show |
| Series | cmd: test: Add operators to deal with hexadecimal numbers. | expand |
On Wed, 11 Mar 2020 at 16:47, Christoph Muellner < christoph.muellner@theobroma-systems.com> wrote: > > The DLUG states the following: > Almost all U-Boot commands expect numbers to be entered in hexadecimal > input format. > > Besides this fact, also most commands output hex values. > > Given this facts, we need to be able to use the information provided > by to/from commands in scripts. The test command does not support > operating on hexadecimal input formats. This leads to very surprising > tests like this (simple_strtol("ff", 10) == 0): > > => if test "ff" -eq 0; then echo "equal"; fi > equal > => > > This patch introduces comparison operators for the test command, > that allow parameters in hexadecimal format. So the test above > can be implemented as follows: > > => if test "ff" -heq 0; then echo "equal"; fi > => > > [1] https://www.denx.de/wiki/view/DULG/UBootCommandLineInterface > > Signed-off-by: Christoph Muellner < christoph.muellner@theobroma-systems.com> > > --- > > cmd/test.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org> This is a good oppty to add a test in cmd_ut.c See ut_assert_nextline() for example. Regards, SImon
On 11. 03. 20 23:46, Christoph Muellner wrote: > The DLUG states the following: > Almost all U-Boot commands expect numbers to be entered in hexadecimal > input format. > > Besides this fact, also most commands output hex values. > > Given this facts, we need to be able to use the information provided > by to/from commands in scripts. The test command does not support > operating on hexadecimal input formats. This leads to very surprising > tests like this (simple_strtol("ff", 10) == 0): > > => if test "ff" -eq 0; then echo "equal"; fi > equal > => > > This patch introduces comparison operators for the test command, > that allow parameters in hexadecimal format. So the test above > can be implemented as follows: > > => if test "ff" -heq 0; then echo "equal"; fi > => > > [1] https://www.denx.de/wiki/view/DULG/UBootCommandLineInterface > > Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> > > --- > > cmd/test.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/cmd/test.c b/cmd/test.c > index 258bfd8806..dbaf23596b 100644 > --- a/cmd/test.c > +++ b/cmd/test.c > @@ -25,6 +25,12 @@ > #define OP_INT_GT 14 > #define OP_INT_GE 15 > #define OP_FILE_EXISTS 16 > +#define OP_HEX_EQ 17 > +#define OP_HEX_NEQ 18 > +#define OP_HEX_LT 19 > +#define OP_HEX_LE 20 > +#define OP_HEX_GT 21 > +#define OP_HEX_GE 22 > > const struct { > int arg; > @@ -48,6 +54,12 @@ const struct { > {0, "-z", OP_STR_EMPTY, 2}, > {0, "-n", OP_STR_NEMPTY, 2}, > {0, "-e", OP_FILE_EXISTS, 4}, > + {1, "-heq", OP_HEX_EQ, 3}, > + {1, "-hne", OP_HEX_NEQ, 3}, > + {1, "-hlt", OP_HEX_LT, 3}, > + {1, "-hle", OP_HEX_LE, 3}, > + {1, "-hgt", OP_HEX_GT, 3}, > + {1, "-hge", OP_HEX_GE, 3}, > }; > > static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > @@ -139,6 +151,30 @@ static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > case OP_FILE_EXISTS: > expr = file_exists(ap[1], ap[2], ap[3], FS_TYPE_ANY); > break; > + case OP_HEX_EQ: > + expr = simple_strtol(ap[0], NULL, 16) == > + simple_strtol(ap[2], NULL, 16); > + break; > + case OP_HEX_NEQ: > + expr = simple_strtol(ap[0], NULL, 16) != > + simple_strtol(ap[2], NULL, 16); > + break; > + case OP_HEX_LT: > + expr = simple_strtol(ap[0], NULL, 16) < > + simple_strtol(ap[2], NULL, 16); > + break; > + case OP_HEX_LE: > + expr = simple_strtol(ap[0], NULL, 16) <= > + simple_strtol(ap[2], NULL, 16); > + break; > + case OP_HEX_GT: > + expr = simple_strtol(ap[0], NULL, 16) > > + simple_strtol(ap[2], NULL, 16); > + break; > + case OP_HEX_GE: > + expr = simple_strtol(ap[0], NULL, 16) >= > + simple_strtol(ap[2], NULL, 16); > + break; > } > > switch (op) { > We have been trying to improve this. Take a look at it. https://lists.denx.de/pipermail/u-boot/2020-March/401745.html It should cover your case. But if you start to mix hex with dec then it will fail. Thanks, Michal
Hi Michal, On 3/12/20 8:38 AM, Michal Simek wrote: > On 11. 03. 20 23:46, Christoph Muellner wrote: >> The DLUG states the following: >> Almost all U-Boot commands expect numbers to be entered in hexadecimal >> input format. >> >> Besides this fact, also most commands output hex values. >> >> Given this facts, we need to be able to use the information provided >> by to/from commands in scripts. The test command does not support >> operating on hexadecimal input formats. This leads to very surprising >> tests like this (simple_strtol("ff", 10) == 0): >> >> => if test "ff" -eq 0; then echo "equal"; fi >> equal >> => >> >> This patch introduces comparison operators for the test command, >> that allow parameters in hexadecimal format. So the test above >> can be implemented as follows: >> >> => if test "ff" -heq 0; then echo "equal"; fi >> => >> >> [1] https://www.denx.de/wiki/view/DULG/UBootCommandLineInterface >> >> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> >> >> --- >> >> cmd/test.c | 36 ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 36 insertions(+) >> >> diff --git a/cmd/test.c b/cmd/test.c >> index 258bfd8806..dbaf23596b 100644 >> --- a/cmd/test.c >> +++ b/cmd/test.c >> @@ -25,6 +25,12 @@ >> #define OP_INT_GT 14 >> #define OP_INT_GE 15 >> #define OP_FILE_EXISTS 16 >> +#define OP_HEX_EQ 17 >> +#define OP_HEX_NEQ 18 >> +#define OP_HEX_LT 19 >> +#define OP_HEX_LE 20 >> +#define OP_HEX_GT 21 >> +#define OP_HEX_GE 22 >> >> const struct { >> int arg; >> @@ -48,6 +54,12 @@ const struct { >> {0, "-z", OP_STR_EMPTY, 2}, >> {0, "-n", OP_STR_NEMPTY, 2}, >> {0, "-e", OP_FILE_EXISTS, 4}, >> + {1, "-heq", OP_HEX_EQ, 3}, >> + {1, "-hne", OP_HEX_NEQ, 3}, >> + {1, "-hlt", OP_HEX_LT, 3}, >> + {1, "-hle", OP_HEX_LE, 3}, >> + {1, "-hgt", OP_HEX_GT, 3}, >> + {1, "-hge", OP_HEX_GE, 3}, >> }; >> >> static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> @@ -139,6 +151,30 @@ static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> case OP_FILE_EXISTS: >> expr = file_exists(ap[1], ap[2], ap[3], FS_TYPE_ANY); >> break; >> + case OP_HEX_EQ: >> + expr = simple_strtol(ap[0], NULL, 16) == >> + simple_strtol(ap[2], NULL, 16); >> + break; >> + case OP_HEX_NEQ: >> + expr = simple_strtol(ap[0], NULL, 16) != >> + simple_strtol(ap[2], NULL, 16); >> + break; >> + case OP_HEX_LT: >> + expr = simple_strtol(ap[0], NULL, 16) < >> + simple_strtol(ap[2], NULL, 16); >> + break; >> + case OP_HEX_LE: >> + expr = simple_strtol(ap[0], NULL, 16) <= >> + simple_strtol(ap[2], NULL, 16); >> + break; >> + case OP_HEX_GT: >> + expr = simple_strtol(ap[0], NULL, 16) > >> + simple_strtol(ap[2], NULL, 16); >> + break; >> + case OP_HEX_GE: >> + expr = simple_strtol(ap[0], NULL, 16) >= >> + simple_strtol(ap[2], NULL, 16); >> + break; >> } >> >> switch (op) { >> > > We have been trying to improve this. Take a look at it. > > https://lists.denx.de/pipermail/u-boot/2020-March/401745.html > > It should cover your case. > But if you start to mix hex with dec then it will fail. thanks for this input. I would prefer to not "guess" the base based on the fact that the string contains [a-f]. Reason is, that I cannot guarantee that all hex numbers contain [a-f]. I want to have something reliably compatible to other commands like setexpr (allowing operations like addition and subtraction, but all in hex format). So I want to strictly interpret all numbers as hex numbers (as stated in the DLUG). Thanks, Christoph
On 12. 03. 20 10:52, Christoph Müllner wrote: > Hi Michal, > > On 3/12/20 8:38 AM, Michal Simek wrote: >> On 11. 03. 20 23:46, Christoph Muellner wrote: >>> The DLUG states the following: >>> Almost all U-Boot commands expect numbers to be entered in hexadecimal >>> input format. >>> >>> Besides this fact, also most commands output hex values. >>> >>> Given this facts, we need to be able to use the information provided >>> by to/from commands in scripts. The test command does not support >>> operating on hexadecimal input formats. This leads to very surprising >>> tests like this (simple_strtol("ff", 10) == 0): >>> >>> => if test "ff" -eq 0; then echo "equal"; fi >>> equal >>> => >>> >>> This patch introduces comparison operators for the test command, >>> that allow parameters in hexadecimal format. So the test above >>> can be implemented as follows: >>> >>> => if test "ff" -heq 0; then echo "equal"; fi >>> => >>> >>> [1] https://www.denx.de/wiki/view/DULG/UBootCommandLineInterface >>> >>> Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> >>> >>> --- >>> >>> cmd/test.c | 36 ++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 36 insertions(+) >>> >>> diff --git a/cmd/test.c b/cmd/test.c >>> index 258bfd8806..dbaf23596b 100644 >>> --- a/cmd/test.c >>> +++ b/cmd/test.c >>> @@ -25,6 +25,12 @@ >>> #define OP_INT_GT 14 >>> #define OP_INT_GE 15 >>> #define OP_FILE_EXISTS 16 >>> +#define OP_HEX_EQ 17 >>> +#define OP_HEX_NEQ 18 >>> +#define OP_HEX_LT 19 >>> +#define OP_HEX_LE 20 >>> +#define OP_HEX_GT 21 >>> +#define OP_HEX_GE 22 >>> >>> const struct { >>> int arg; >>> @@ -48,6 +54,12 @@ const struct { >>> {0, "-z", OP_STR_EMPTY, 2}, >>> {0, "-n", OP_STR_NEMPTY, 2}, >>> {0, "-e", OP_FILE_EXISTS, 4}, >>> + {1, "-heq", OP_HEX_EQ, 3}, >>> + {1, "-hne", OP_HEX_NEQ, 3}, >>> + {1, "-hlt", OP_HEX_LT, 3}, >>> + {1, "-hle", OP_HEX_LE, 3}, >>> + {1, "-hgt", OP_HEX_GT, 3}, >>> + {1, "-hge", OP_HEX_GE, 3}, >>> }; >>> >>> static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >>> @@ -139,6 +151,30 @@ static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >>> case OP_FILE_EXISTS: >>> expr = file_exists(ap[1], ap[2], ap[3], FS_TYPE_ANY); >>> break; >>> + case OP_HEX_EQ: >>> + expr = simple_strtol(ap[0], NULL, 16) == >>> + simple_strtol(ap[2], NULL, 16); >>> + break; >>> + case OP_HEX_NEQ: >>> + expr = simple_strtol(ap[0], NULL, 16) != >>> + simple_strtol(ap[2], NULL, 16); >>> + break; >>> + case OP_HEX_LT: >>> + expr = simple_strtol(ap[0], NULL, 16) < >>> + simple_strtol(ap[2], NULL, 16); >>> + break; >>> + case OP_HEX_LE: >>> + expr = simple_strtol(ap[0], NULL, 16) <= >>> + simple_strtol(ap[2], NULL, 16); >>> + break; >>> + case OP_HEX_GT: >>> + expr = simple_strtol(ap[0], NULL, 16) > >>> + simple_strtol(ap[2], NULL, 16); >>> + break; >>> + case OP_HEX_GE: >>> + expr = simple_strtol(ap[0], NULL, 16) >= >>> + simple_strtol(ap[2], NULL, 16); >>> + break; >>> } >>> >>> switch (op) { >>> >> >> We have been trying to improve this. Take a look at it. >> >> https://lists.denx.de/pipermail/u-boot/2020-March/401745.html >> >> It should cover your case. >> But if you start to mix hex with dec then it will fail. > > thanks for this input. > > I would prefer to not "guess" the base based on the fact that the string contains [a-f]. > Reason is, that I cannot guarantee that all hex numbers contain [a-f]. > > I want to have something reliably compatible to other commands like setexpr > (allowing operations like addition and subtraction, but all in hex format). > So I want to strictly interpret all numbers as hex numbers (as stated in the DLUG). Yes I am aware about it. Also someone mentioned to take a look at itest which is using hex as base all the time. M
diff --git a/cmd/test.c b/cmd/test.c index 258bfd8806..dbaf23596b 100644 --- a/cmd/test.c +++ b/cmd/test.c @@ -25,6 +25,12 @@ #define OP_INT_GT 14 #define OP_INT_GE 15 #define OP_FILE_EXISTS 16 +#define OP_HEX_EQ 17 +#define OP_HEX_NEQ 18 +#define OP_HEX_LT 19 +#define OP_HEX_LE 20 +#define OP_HEX_GT 21 +#define OP_HEX_GE 22 const struct { int arg; @@ -48,6 +54,12 @@ const struct { {0, "-z", OP_STR_EMPTY, 2}, {0, "-n", OP_STR_NEMPTY, 2}, {0, "-e", OP_FILE_EXISTS, 4}, + {1, "-heq", OP_HEX_EQ, 3}, + {1, "-hne", OP_HEX_NEQ, 3}, + {1, "-hlt", OP_HEX_LT, 3}, + {1, "-hle", OP_HEX_LE, 3}, + {1, "-hgt", OP_HEX_GT, 3}, + {1, "-hge", OP_HEX_GE, 3}, }; static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) @@ -139,6 +151,30 @@ static int do_test(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) case OP_FILE_EXISTS: expr = file_exists(ap[1], ap[2], ap[3], FS_TYPE_ANY); break; + case OP_HEX_EQ: + expr = simple_strtol(ap[0], NULL, 16) == + simple_strtol(ap[2], NULL, 16); + break; + case OP_HEX_NEQ: + expr = simple_strtol(ap[0], NULL, 16) != + simple_strtol(ap[2], NULL, 16); + break; + case OP_HEX_LT: + expr = simple_strtol(ap[0], NULL, 16) < + simple_strtol(ap[2], NULL, 16); + break; + case OP_HEX_LE: + expr = simple_strtol(ap[0], NULL, 16) <= + simple_strtol(ap[2], NULL, 16); + break; + case OP_HEX_GT: + expr = simple_strtol(ap[0], NULL, 16) > + simple_strtol(ap[2], NULL, 16); + break; + case OP_HEX_GE: + expr = simple_strtol(ap[0], NULL, 16) >= + simple_strtol(ap[2], NULL, 16); + break; } switch (op) {
The DLUG states the following: Almost all U-Boot commands expect numbers to be entered in hexadecimal input format. Besides this fact, also most commands output hex values. Given this facts, we need to be able to use the information provided by to/from commands in scripts. The test command does not support operating on hexadecimal input formats. This leads to very surprising tests like this (simple_strtol("ff", 10) == 0): => if test "ff" -eq 0; then echo "equal"; fi equal => This patch introduces comparison operators for the test command, that allow parameters in hexadecimal format. So the test above can be implemented as follows: => if test "ff" -heq 0; then echo "equal"; fi => [1] https://www.denx.de/wiki/view/DULG/UBootCommandLineInterface Signed-off-by: Christoph Muellner <christoph.muellner@theobroma-systems.com> --- cmd/test.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)