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