Re: [PATCH v1 1/1] cmd: setexpr: add dec operation for converting variable to decimal

["Roland Gaudig (OSS)" <roland.gaudig-oss@weidmueller.com>]

Hi Simon,

On Tue, 22. Jun 2021 at 21:25, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Roland,
> 
> On Tue, 22 Jun 2021 at 07:51, <roland.gaudig-oss@weidmueller.com> wrote:
>>
>> From: Roland Gaudig <roland.gaudig@weidmueller.com>
>>
>> This patch extends the setexpr command with a dec operator to
>> convert an input value to decimal.
>>
>> Signed-off-by: Roland Gaudig <roland.gaudig@weidmueller.com>
>> ---
>>
>>  cmd/setexpr.c | 16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/cmd/setexpr.c b/cmd/setexpr.c
>> index e828be3970..2d4bee2182 100644
>> --- a/cmd/setexpr.c
>> +++ b/cmd/setexpr.c
>> @@ -370,15 +370,16 @@ static int do_setexpr(struct cmd_tbl *cmdtp, int 
> flag, int argc,
>>         int w;
>>
>>         /*
>> -        * We take 3, 5, or 6 arguments:
>> +        * We take 3, 4, 5, or 6 arguments:
>>          * 3 : setexpr name value
>> +        * 4 : setexpr name dec value
>>          * 5 : setexpr name val1 op val2
>>          *     setexpr name [g]sub r s
>>          * 6 : setexpr name [g]sub r s t
>>          */
>>
>>         /* > 6 already tested by max command args */
>> -       if ((argc < 3) || (argc == 4))
>> +       if (argc < 3)
>>                 return CMD_RET_USAGE;
>>
>>         w = cmd_get_data_size(argv[0], 4);
>> @@ -398,6 +399,13 @@ static int do_setexpr(struct cmd_tbl *cmdtp, int flag, 
> int argc,
>>                 return ret;
>>         }
>>
>> +       /* hexadecimal to decimal conversion: "setexpr name dec value" */
>> +       if (argc == 4 && (strcmp(argv[2], "dec") == 0)) {
>> +               w = cmd_get_data_size(argv[3], 4);
>> +               a = get_arg(argv[3], w);
>> +               return env_set_ulong(argv[1], a);
>> +       }
>> +
>>         /* 5 or 6 args (6 args only with [g]sub) */
>>  #ifdef CONFIG_REGEX
>>         /*
>> @@ -515,4 +523,8 @@ U_BOOT_CMD(
>>         "setexpr name sub r s [t]\n"
>>         "    - Just like gsub(), but replace only the first matching 
> substring"
>>  #endif
>> +       "\n"
>> +       "setexpr name dec [*]value\n"
>> +       "    - set environment variable 'name' to the result of the 
> decimal\n"
>> +       "      conversion of [*]value.\n"
>>  );
>> --
>> 2.25.1
>>
> 
> This seems reasonable to me.
> 
> I have been thinking of introducing a prefix for decimal, perhaps
> 0m123 ? ('m' for deciMal).

This 0x prefix would then also be parsed by get_arg(), as it does currently
with the 0x prefix?

For my application as input to the Linux bootargs it is important, that the
resulting text stored in the variable after running setexpr has no prefix
at all.

> Can you please add a test for this in test//cmd/setexpr.c and also,
> how about adding something in doc/usage?

Yes, I will add a test and a description to doc/usage.

Regards,
Roland