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

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

[Matthew Bright]

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@alliedtelesis.co.nz>
Reviewed-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
Reviewed-by: Chris Packham <chris.packham@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;
-
 	switch (op) {
 	case env_op_create:
 	case env_op_overwrite:
@@ -229,9 +226,6 @@ U_BOOT_ENV_CALLBACK(bootfile, on_bootfile);
 static int on_ipaddr(const char *name, const char *value, enum env_op op,
 	int flags)
 {
-	if (flags & H_PROGRAMMATIC)
-		return 0;
-
 	net_ip = string_to_ip(value);
 
 	return 0;
@@ -241,9 +235,6 @@ U_BOOT_ENV_CALLBACK(ipaddr, on_ipaddr);
 static int on_gatewayip(const char *name, const char *value, enum env_op op,
 	int flags)
 {
-	if (flags & H_PROGRAMMATIC)
-		return 0;
-
 	net_gateway = string_to_ip(value);
 
 	return 0;
@@ -253,9 +244,6 @@ U_BOOT_ENV_CALLBACK(gatewayip, on_gatewayip);
 static int on_netmask(const char *name, const char *value, enum env_op op,
 	int flags)
 {
-	if (flags & H_PROGRAMMATIC)
-		return 0;
-
 	net_netmask = string_to_ip(value);
 
 	return 0;
@@ -265,9 +253,6 @@ U_BOOT_ENV_CALLBACK(netmask, on_netmask);
 static int on_serverip(const char *name, const char *value, enum env_op op,
 	int flags)
 {
-	if (flags & H_PROGRAMMATIC)
-		return 0;
-
 	net_server_ip = string_to_ip(value);
 
 	return 0;
@@ -277,9 +262,6 @@ U_BOOT_ENV_CALLBACK(serverip, on_serverip);
 static int on_nvlan(const char *name, const char *value, enum env_op op,
 	int flags)
 {
-	if (flags & H_PROGRAMMATIC)
-		return 0;
-
 	net_native_vlan = string_to_vlan(value);
 
 	return 0;
@@ -289,9 +271,6 @@ U_BOOT_ENV_CALLBACK(nvlan, on_nvlan);
 static int on_vlan(const char *name, const char *value, enum env_op op,
 	int flags)
 {
-	if (flags & H_PROGRAMMATIC)
-		return 0;
-
 	net_our_vlan = string_to_vlan(value);
 
 	return 0;
@@ -302,9 +281,6 @@ U_BOOT_ENV_CALLBACK(vlan, on_vlan);
 static int on_dnsip(const char *name, const char *value, enum env_op op,
 	int flags)
 {
-	if (flags & H_PROGRAMMATIC)
-		return 0;
-
 	net_dns_server = string_to_ip(value);
 
 	return 0;
-- 
2.8.4

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

[Joe Hershberger]

On Thu, Jun 9, 2016 at 8:40 PM, Matthew Bright
<matthew.bright@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@alliedtelesis.co.nz>
> Reviewed-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
> Reviewed-by: Chris Packham <chris.packham@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?

Thanks,
-Joe

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

[Chris Packham]

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@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@alliedtelesis.co.nz>
>> Reviewed-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
>> Reviewed-by: Chris Packham <chris.packham@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?

[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@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@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@alliedtelesis.co.nz>
>>> Reviewed-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
>>> Reviewed-by: Chris Packham <chris.packham@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

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

[Chris Packham]

On 06/14/2016 06:34 AM, Joe Hershberger wrote:
> Hi Chris,
>
> On Sun, Jun 12, 2016 at 3:58 PM, Chris Packham
> <Chris.Packham@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@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@alliedtelesis.co.nz>
>>>> Reviewed-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
>>>> Reviewed-by: Chris Packham <chris.packham@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.
>

That's what I was thinking. Because setenv is one of the exported 
functions for standalone applications I was wondering if instead of 
setenv() passing H_PROGRAMMATIC we add prog_setenv() (naming things is 
hard) for the net use-case since that is the only thing that currently 
checks H_PROGRAMMATIC.

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

[Joe Hershberger]

Hi Chris,

On Mon, Jun 13, 2016 at 4:13 PM, Chris Packham
<Chris.Packham@alliedtelesis.co.nz> wrote:
> On 06/14/2016 06:34 AM, Joe Hershberger wrote:
>> Hi Chris,
>>
>> On Sun, Jun 12, 2016 at 3:58 PM, Chris Packham
>> <Chris.Packham@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@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@alliedtelesis.co.nz>
>>>>> Reviewed-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
>>>>> Reviewed-by: Chris Packham <chris.packham@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.
>>
>
> That's what I was thinking. Because setenv is one of the exported
> functions for standalone applications I was wondering if instead of
> setenv() passing H_PROGRAMMATIC we add prog_setenv() (naming things is
> hard) for the net use-case since that is the only thing that currently
> checks H_PROGRAMMATIC.

That might be OK. The only reservation I have about it is that the
setenv() function is generally a programmatic operation since only C
code can get to it. Only in the case where you are implementing some
more complex interaction (like your menu) is it not actually
programmatic. I just worry about it being misleading in the future.

-Joe

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

[Chris Packham]

On 06/14/2016 10:19 AM, Joe Hershberger wrote:
> Hi Chris,
>
> On Mon, Jun 13, 2016 at 4:13 PM, Chris Packham
> <Chris.Packham@alliedtelesis.co.nz> wrote:
>> On 06/14/2016 06:34 AM, Joe Hershberger wrote:
>>> Hi Chris,
>>>
>>> On Sun, Jun 12, 2016 at 3:58 PM, Chris Packham
>>> <Chris.Packham@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@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@alliedtelesis.co.nz>
>>>>>> Reviewed-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
>>>>>> Reviewed-by: Chris Packham <chris.packham@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.
>>>
>>
>> That's what I was thinking. Because setenv is one of the exported
>> functions for standalone applications I was wondering if instead of
>> setenv() passing H_PROGRAMMATIC we add prog_setenv() (naming things is
>> hard) for the net use-case since that is the only thing that currently
>> checks H_PROGRAMMATIC.
>
> That might be OK. The only reservation I have about it is that the
> setenv() function is generally a programmatic operation since only C
> code can get to it. Only in the case where you are implementing some
> more complex interaction (like your menu) is it not actually
> programmatic. I just worry about it being misleading in the future.
>

Agreed. My initial reaction was that our menu should be treated like 
H_INTERACTIVE but there wasn't an easy way to achieve this.

Do you have any feel for the direction of H_PROGRAMMATIC is going? Are 
we going to see more environment variables in other parts of the code 
that will get similar treatment.

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

[Joe Hershberger]

Hi Chris,

On Mon, Jun 13, 2016 at 5:52 PM, Chris Packham
<Chris.Packham@alliedtelesis.co.nz> wrote:
> On 06/14/2016 10:19 AM, Joe Hershberger wrote:
>> Hi Chris,
>>
>> On Mon, Jun 13, 2016 at 4:13 PM, Chris Packham
>> <Chris.Packham@alliedtelesis.co.nz> wrote:
>>> On 06/14/2016 06:34 AM, Joe Hershberger wrote:
>>>> Hi Chris,
>>>>
>>>> On Sun, Jun 12, 2016 at 3:58 PM, Chris Packham
>>>> <Chris.Packham@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@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@alliedtelesis.co.nz>
>>>>>>> Reviewed-by: Hamish Martin <hamish.martin@alliedtelesis.co.nz>
>>>>>>> Reviewed-by: Chris Packham <chris.packham@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.
>>>>
>>>
>>> That's what I was thinking. Because setenv is one of the exported
>>> functions for standalone applications I was wondering if instead of
>>> setenv() passing H_PROGRAMMATIC we add prog_setenv() (naming things is
>>> hard) for the net use-case since that is the only thing that currently
>>> checks H_PROGRAMMATIC.
>>
>> That might be OK. The only reservation I have about it is that the
>> setenv() function is generally a programmatic operation since only C
>> code can get to it. Only in the case where you are implementing some
>> more complex interaction (like your menu) is it not actually
>> programmatic. I just worry about it being misleading in the future.
>>
>
> Agreed. My initial reaction was that our menu should be treated like
> H_INTERACTIVE but there wasn't an easy way to achieve this.
>
> Do you have any feel for the direction of H_PROGRAMMATIC is going? Are
> we going to see more environment variables in other parts of the code
> that will get similar treatment.

Given that I implemented the code in question, I can't say I can give
an unbiased opinion about the direction. I would tend toward using
this same paradigm in other places. :)

I can prolly send an RFC tomorrow that shows what I have in mind for
addressing this.

Cheers,
-Joe