[PATCH 0/3] add "call" command

[Rasmus Villemoes]

On 25/09/2020 13.52, Heinrich Schuchardt wrote:
> On 25.09.20 13:19, Rasmus Villemoes wrote:
>> This adds a way to call a "function" defined in the environment with
>> arguments. I.e., whereas
>>
>>   run foo
>>
>> requires one to set the (shell or environment) variables referenced
>> from foo beforehand, with this one can instead do
>>
>>   call foo arg1 arg2 arg3
>>
>> and use $1... up to $9 in the definition of foo. $# is set so foo can
>> make decisions based on that, and ${3:-default} works as expected.
>>
>> As I write in patch 2, it should be possible to get rid of the "call"
>> and simply allow
>>
>>   foo arg1 arg2 arg3
>>
>> i.e. if the search for a command named foo fails, try an environment
>> variable by that name and do it as "call". But that change of
>> behaviour, I think, requires a separate opt-in config knob, and can be
>> done later if someone actually wants that.
> 
> If the behavior is configurable, this will result in users complaining
> that a script which they copied does not run on another machine. Please,
> do not introduce any configurability here.

OK, but I'm actually not intending to add that functionality at all, I
was merely mentioning it if somebody would like it. But note that the
same argument can be used for any script that uses any command (or other
functionality) which is config-dependent - if I copy some snippet that
uses setexpr, that won't work on another machine that doesn't have
CONFIG_CMD_SETEXPR.

> Further we cannot first introduce a command call and then eliminate it
> due to backward compatibility. We should decide on the final version
> beforehand.
> 
> In the Linux world you can override a command using an alias. So I am
> not sure if a built in command should take precedence over a variable of
> the same name or the other way round.

POSIX(-like) shells have "command":

  The command utility shall cause the shell to treat the arguments as a
simple command, suppressing the shell function lookup

So I'd be inclined to make the normal commands have precedence, given
that there's a "call " that can be used to invoke the "function" variant
rather than the "builtin command" variant. But it's all moot if nobody
actually wants to be able to avoid the "call ".

> Consider that we already have two types of variables in the shell. Those
> that are in the environment and those that are not. Here the environment
> variables take precedence.
> 
> Try
> 
>     setenv a;a=4;echo $a;setenv a 5;echo $a;setenv a;echo $a

I know. So if you do 'env set 1 haha"', ${1} will not work (though I
think bare $1 will) in called functions. But nobody sets such
environment variables.

While I was trying to figure out how to implement this, I stumbled on
some code that tries to prevent the above (or rather, the converse:
setting a shell variable when an env variable of that name exists). But
that code is buggy and hence dead because it does the lookup on the
whole "a=4" string, before splitting on =. So currently we get

=> env set foo abc
=> foo=x
=>

moving the check:

--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -2185,14 +2185,6 @@ int set_local_var(const char *s, int flg_export)

        name=strdup(s);

-#ifdef __U_BOOT__
-       if (env_get(name) != NULL) {
-               printf ("ERROR: "
-                               "There is a global environment variable
with the same name.\n");
-               free(name);
-               return -1;
-       }
-#endif
        /* Assume when we enter this function that we are already in
         * NAME=VALUE format.  So the first order of business is to
         * split 's' on the '=' into 'name' and 'value' */
@@ -2203,6 +2195,15 @@ int set_local_var(const char *s, int flg_export)
        }
        *value++ = 0;

+#ifdef __U_BOOT__
+       if (env_get(name) != NULL) {
+               printf ("ERROR: "
+                               "There is a global environment variable
with the same name.\n");
+               free(name);
+               return -1;
+       }
+#endif
+

we get

=> env set foo abc
=> foo=x
ERROR: There is a global environment variable with the same name.
=>

But that code is ancient, so I don't know if it should be fixed to work
as clearly intended, or one should just delete it to remove a few bytes
of dead .text. In any case it does nothing to prevent setting an env
variable with the same name as an existing shell variable.

Rasmus