diff mbox series

cmd: test: Add operators to deal with hexadecimal numbers.

Message ID 20200311224652.16900-1-christoph.muellner@theobroma-systems.com
State New
Headers show
Series cmd: test: Add operators to deal with hexadecimal numbers. | expand

Commit Message

=?UTF-8?Q?Christoph_M=c3=bcllner?= March 11, 2020, 10:46 p.m. UTC
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 at theobroma-systems.com>

---

 cmd/test.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Comments

Simon Glass March 12, 2020, 3:22 a.m. UTC | #1
On Wed, 11 Mar 2020 at 16:47, Christoph Muellner <
christoph.muellner at 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 at theobroma-systems.com>
>
> ---
>
>  cmd/test.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)

Reviewed-by: Simon Glass <sjg at chromium.org>

This is a good oppty to add a test in cmd_ut.c

See ut_assert_nextline() for example.

Regards,
SImon
Michal Simek March 12, 2020, 7:38 a.m. UTC | #2
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 at 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
=?UTF-8?Q?Christoph_M=c3=bcllner?= March 12, 2020, 9:52 a.m. UTC | #3
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 at 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
Michal Simek March 12, 2020, 9:59 a.m. UTC | #4
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 at 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 mbox series

Patch

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) {