[U-Boot] [PATCH] net: Allow setenv to set net global variables

[Joe Hershberger]

Hi Chris,

On Sun, Jun 12, 2016 at 3:58 PM, Chris Packham
<Chris.Packham at alliedtelesis.co.nz> wrote:
> Hi Joe,
>
> On 06/11/2016 03:56 AM, Joe Hershberger wrote:
>> On Thu, Jun 9, 2016 at 8:40 PM, Matthew Bright
>> <matthew.bright at alliedtelesis.co.nz> wrote:
>>> The patch fd3056337e6fcc introduces env callbacks to several of the net
>>> related env variables. These callbacks are responsible for updating the
>>> corresponding global variables internal to the net source code. However
>>> this behavior will be skipped if the source of the callbacks originated
>>> from setenv. This is based on the assumption that all current instances
>>> of setenv are invoked using the same global variables that the callback
>>> will eventually write to; therefore there is no need set them to the
>>> same value.
>>>
>>> As setenv is a public interface this assumption may not always hold. In
>>> our usage case we implement a user facing menu system for configuration
>>> of networking parameters. This ultimately lead to calling setenv rather
>>> than through the traditional interactive command line parser do_env_set.
>>> Therefore, in our usage case, setenv can be called for an "interactive"
>>> case. Consequently, the early return for non-interactive invocation are
>>> now removed and any call to setenv will update the corresponding states
>>> internal to the net source code as expected.
>>>
>>> Signed-off-by: Matthew Bright <matthew.bright at alliedtelesis.co.nz>
>>> Reviewed-by: Hamish Martin <hamish.martin at alliedtelesis.co.nz>
>>> Reviewed-by: Chris Packham <chris.packham at alliedtelesis.co.nz>
>>> ---
>>>   net/net.c | 24 ------------------------
>>>   1 file changed, 24 deletions(-)
>>>
>>> diff --git a/net/net.c b/net/net.c
>>> index 1e1d23d..726b0f0 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -209,9 +209,6 @@ int __maybe_unused net_busy_flag;
>>>   static int on_bootfile(const char *name, const char *value, enum env_op op,
>>>          int flags)
>>>   {
>>> -       if (flags & H_PROGRAMMATIC)
>>> -               return 0;
>>> -
>>
>> Why can't you just change your menu to call the API that is
>> interactive instead of setenv?
>
> Which API are you referring to? _do_env_set() is static so the only
> public api would be run_command("setenv ipaddr ...") or have I missed
> something?

Yes, that's what I was referring to.

Another option would be to add an explicit function that provides this
directly. Maybe even make a generic version that accepts a flags
parameter, then implement the existing function as a call to this new
function which passes in a "programmatic" flag.

-Joe