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