| Message ID | 1479292229-17256-7-git-send-email-bernhard.nortmann@web.de |
|---|---|
| State | RFC |
| Delegated to: | Joe Hershberger |
| Headers | show |
Hi Bernhard, On 16 November 2016 at 03:30, Bernhard Nortmann <bernhard.nortmann@web.de> wrote: > Like setenv(), but automatically marks the entry as "don't export". > > Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de> > --- > > Changes in v2: None > > cmd/nvedit.c | 21 +++++++++++++++++++++ > include/common.h | 1 + > 2 files changed, 22 insertions(+) > Reviewed-by: Simon Glass <sjg@chromium.org> But see below. > diff --git a/cmd/nvedit.c b/cmd/nvedit.c > index 9a78e1d..fbed3df 100644 > --- a/cmd/nvedit.c > +++ b/cmd/nvedit.c > @@ -300,6 +300,27 @@ int setenv(const char *varname, const char *varvalue) > } > > /** > + * Set a "transient" environment variable > + * > + * Like setenv(), but this automatically marks the > + * resulting entry as transient (= "do not export"). > + */ > +int setenv_transient(const char *varname, const char *varvalue) > +{ > + int rc = setenv(varname, varvalue); > + if (rc == 0) { I think returning the error right away is better here. if (rc) return rc; > + ENTRY e, *ep; > + > + e.key = varname; > + e.data = NULL; > + hsearch_r(e, FIND, &ep, &env_htab, 0); > + if (ep) > + ep->flags |= ENV_FLAGS_VARACCESS_PREVENT_EXPORT; > + } > + return rc; return 0; > +} > + > +/** > * Set an environment variable to an integer value > * > * @param varname Environment variable to set > diff --git a/include/common.h b/include/common.h > index a8d833b..c0fd319 100644 > --- a/include/common.h > +++ b/include/common.h > @@ -392,6 +392,7 @@ ulong getenv_hex(const char *varname, ulong default_val); > int getenv_yesno(const char *var); > int saveenv (void); > int setenv (const char *, const char *); > +int setenv_transient(const char *, const char *); Please add a function comment and parameter names. > int setenv_ulong(const char *varname, ulong value); > int setenv_hex(const char *varname, ulong value); > /** > -- > 2.7.3 > Regards, SImon
Hi Simon! Am 19.11.2016 um 14:47 schrieb Simon Glass: > Hi Bernhard, > > On 16 November 2016 at 03:30, Bernhard Nortmann > <bernhard.nortmann@web.de> wrote: >> Like setenv(), but automatically marks the entry as "don't export". >> >> Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de> >> --- >> >> Changes in v2: None >> >> cmd/nvedit.c | 21 +++++++++++++++++++++ >> include/common.h | 1 + >> 2 files changed, 22 insertions(+) >> > Reviewed-by: Simon Glass <sjg@chromium.org> > > But see below. > > [...] > I think returning the error right away is better here. > > if (rc) > return rc; > > [...] > return 0; Makes sense, I'll change that. > [...] > Please add a function comment and parameter names. There are inconsistent coding styles here. getenv_hex() has a documentation comment in include/common.h, getenv_yesno() only a standard comment. The inline function setenv_addr() is both documented and implemented there. setenv() has no comments at all. Other functions have their documentation (next to their implementation) in cmd/nvedit.c - which is what I usually prefer over 'inflating' the header, too. For setenv_transient() I oriented myself at the 'parent' function setenv(), but added a documentation comment in cmd/nvedit.c. I'm fine with adding @param and @return descriptions, and parameter names to include/common.h. If you insist, I could also move the description over there. Regards, B. Nortmann
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 9a78e1d..fbed3df 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -300,6 +300,27 @@ int setenv(const char *varname, const char *varvalue) } /** + * Set a "transient" environment variable + * + * Like setenv(), but this automatically marks the + * resulting entry as transient (= "do not export"). + */ +int setenv_transient(const char *varname, const char *varvalue) +{ + int rc = setenv(varname, varvalue); + if (rc == 0) { + ENTRY e, *ep; + + e.key = varname; + e.data = NULL; + hsearch_r(e, FIND, &ep, &env_htab, 0); + if (ep) + ep->flags |= ENV_FLAGS_VARACCESS_PREVENT_EXPORT; + } + return rc; +} + +/** * Set an environment variable to an integer value * * @param varname Environment variable to set diff --git a/include/common.h b/include/common.h index a8d833b..c0fd319 100644 --- a/include/common.h +++ b/include/common.h @@ -392,6 +392,7 @@ ulong getenv_hex(const char *varname, ulong default_val); int getenv_yesno(const char *var); int saveenv (void); int setenv (const char *, const char *); +int setenv_transient(const char *, const char *); int setenv_ulong(const char *varname, ulong value); int setenv_hex(const char *varname, ulong value); /**
Like setenv(), but automatically marks the entry as "don't export". Signed-off-by: Bernhard Nortmann <bernhard.nortmann@web.de> --- Changes in v2: None cmd/nvedit.c | 21 +++++++++++++++++++++ include/common.h | 1 + 2 files changed, 22 insertions(+)