diff mbox series

common: cli_hush: Restore clear local variable support

Message ID 20230331144328.30030-1-stefan.herbrechtsmeier-oss@weidmueller.com
State Superseded
Delegated to: Tom Rini
Headers show
Series common: cli_hush: Restore clear local variable support | expand

Commit Message

Stefan Herbrechtsmeier March 31, 2023, 2:43 p.m. UTC 
From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

The u-boot hush shell doesn’t support the unset command to clear a
variable and therefore an empty value ("c=") should be a valid value
for the set_local_var function to clear the variable. This partial
reverts commit aa722529635c ("common: cli_hush: avoid dead code") and
only checks for a `=` in the string. Additionally explicit call the
unset_local_var function to remove the variable if the value is empty.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

---

 common/cli_hush.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

--
2.30.2

Comments

Simon Glass April 1, 2023, 6:31 a.m. UTC | #1 
Hi Stefan,

On Sat, 1 Apr 2023 at 03:43, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> The u-boot hush shell doesn’t support the unset command to clear a
> variable and therefore an empty value ("c=") should be a valid value
> for the set_local_var function to clear the variable. This partial
> reverts commit aa722529635c ("common: cli_hush: avoid dead code") and
> only checks for a `=` in the string. Additionally explicit call the
> unset_local_var function to remove the variable if the value is empty.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> ---
>
>  common/cli_hush.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/common/cli_hush.c b/common/cli_hush.c
> index 1ad7a509df..c3f7dd12a0 100644
> --- a/common/cli_hush.c
> +++ b/common/cli_hush.c
> @@ -2171,12 +2171,18 @@ int set_local_var(const char *s, int flg_export)
>          * NAME=VALUE format.  So the first order of business is to
>          * split 's' on the '=' into 'name' and 'value' */
>         value = strchr(name, '=');
> -       if (value == NULL || *(value + 1) == 0) {
> +       if (value == NULL) {

if (!value)

>                 free(name);
>                 return -1;
>         }
>         *value++ = 0;
>
> +       if (*value == 0) {

if (!*value)

> +               unset_local_var(name);
> +               free(name);
> +               return 0;
> +       }
> +
>         for(cur = top_vars; cur; cur = cur->next) {
>                 if(strcmp(cur->name, name)==0)
>                         break;
> --
> 2.30.2

I think this should have a test, e.g. in
test/py/tests/test_hush_if_test.py or perhaps a C test?

Regards,
Simon
Stefan Herbrechtsmeier April 3, 2023, 9:35 a.m. UTC | #2 
Hi Simon,

Am 01.04.2023 um 08:31 schrieb Simon Glass:

> Hi Stefan,
>
> On Sat, 1 Apr 2023 at 03:43, Stefan Herbrechtsmeier
> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>
>> The u-boot hush shell doesn’t support the unset command to clear a
>> variable and therefore an empty value ("c=") should be a valid value
>> for the set_local_var function to clear the variable. This partial
>> reverts commit aa722529635c ("common: cli_hush: avoid dead code") and
>> only checks for a `=` in the string. Additionally explicit call the
>> unset_local_var function to remove the variable if the value is empty.
>>
>> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>
>> ---
>>
>>   common/cli_hush.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/cli_hush.c b/common/cli_hush.c
>> index 1ad7a509df..c3f7dd12a0 100644
>> --- a/common/cli_hush.c
>> +++ b/common/cli_hush.c
>> @@ -2171,12 +2171,18 @@ int set_local_var(const char *s, int flg_export)
>>           * NAME=VALUE format.  So the first order of business is to
>>           * split 's' on the '=' into 'name' and 'value' */
>>          value = strchr(name, '=');
>> -       if (value == NULL || *(value + 1) == 0) {
>> +       if (value == NULL) {
> if (!value)

I reuse the existing style of the file. Should I always use the kernel /
u-boot style for changes?

>>                  free(name);
>>                  return -1;
>>          }
>>          *value++ = 0;
>>
>> +       if (*value == 0) {
> if (!*value)
>
>> +               unset_local_var(name);
>> +               free(name);
>> +               return 0;
>> +       }
>> +
>>          for(cur = top_vars; cur; cur = cur->next) {
>>                  if(strcmp(cur->name, name)==0)
>>                          break;
>> --
>> 2.30.2
> I think this should have a test, e.g. in
> test/py/tests/test_hush_if_test.py or perhaps a C test?

I will extend the python test.

Regards
   Stefan
diff mbox series

Patch

diff --git a/common/cli_hush.c b/common/cli_hush.c
index 1ad7a509df..c3f7dd12a0 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -2171,12 +2171,18 @@  int set_local_var(const char *s, int flg_export)
         * NAME=VALUE format.  So the first order of business is to
         * split 's' on the '=' into 'name' and 'value' */
        value = strchr(name, '=');
-       if (value == NULL || *(value + 1) == 0) {
+       if (value == NULL) {
                free(name);
                return -1;
        }
        *value++ = 0;

+       if (*value == 0) {
+               unset_local_var(name);
+               free(name);
+               return 0;
+       }
+
        for(cur = top_vars; cur; cur = cur->next) {
                if(strcmp(cur->name, name)==0)
                        break;