| Message ID | 20210622135042.133904-2-roland.gaudig-oss@weidmueller.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | cmd: setexpr: add dec operation for converting variable to decimal | expand |
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). Can you please add a test for this in test//cmd/setexpr.c and also, how about adding something in doc/usage? Regards, Simon
On 6/22/21 3:25 PM, Simon Glass 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). Perhaps 0d123? Though I would prefer to remove many of the implicit assumptions of hex input. --Sean > > Can you please add a test for this in test//cmd/setexpr.c and also, > how about adding something in doc/usage? > > Regards, > Simon >
Hi Sean, On Tue, 22 Jun 2021 at 13:30, Sean Anderson <sean.anderson@seco.com> wrote: > > > > On 6/22/21 3:25 PM, Simon Glass 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). > > Perhaps 0d123? Though I would prefer to remove many of the implicit > assumptions of hex input. Right, we can't use 'd' because it is valid hex. I believe hex is the right default. We just need an easy way to use decimal. - Simon > > --Sean > > > > > Can you please add a test for this in test//cmd/setexpr.c and also, > > how about adding something in doc/usage? > > > > Regards, > > Simon > >
Dear Roland, In message <20210622135042.133904-2-roland.gaudig-oss@weidmueller.com> you wrote: > > This patch extends the setexpr command with a dec operator to > convert an input value to decimal. ... > + /* 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); > + } Should there not be a test for 4 arguments and the third _not_ being "dec" ? Like "setexpr foo hex 42" ? Best regards, Wolfgang Denk
Dear Simon, In message <CAPnjgZ13=2SCsZC1Y+m-HZOiBhpGiEFe2dRWwK9sAZHutgaO7Q@mail.gmail.com> you wrote: > > > > 0m123 ? ('m' for deciMal). > > > > Perhaps 0d123? Though I would prefer to remove many of the implicit > > assumptions of hex input. > > Right, we can't use 'd' because it is valid hex. > > I believe hex is the right default. We just need an easy way to use decimal. Maybe we should make this more general and support an even wirder range of formats? Instead of just converting to decimal, we could pass a format string for sprintf() ? Like: # setexpr foo fmt %d $value ? Best regards, Wolfgang Denk
On Wed, Jun 23, 2021 at 08:08:50AM +0200, Wolfgang Denk wrote: > Dear Simon, > > In message <CAPnjgZ13=2SCsZC1Y+m-HZOiBhpGiEFe2dRWwK9sAZHutgaO7Q@mail.gmail.com> you wrote: > > > > > > 0m123 ? ('m' for deciMal). > > > > > > Perhaps 0d123? Though I would prefer to remove many of the implicit > > > assumptions of hex input. > > > > Right, we can't use 'd' because it is valid hex. > > > > I believe hex is the right default. We just need an easy way to use decimal. > > Maybe we should make this more general and support an even wirder > range of formats? Instead of just converting to decimal, we could > pass a format string for sprintf() ? > > Like: > > # setexpr foo fmt %d $value I like this idea as well.
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" );