Hi Simon!
Am 19.11.2016 um 14:47 schrieb Simon Glass:
Hi Bernhard,
On 16 November 2016 at 03:30, Bernhard Nortmann
<bernhard.nortm...@web.de> wrote:
Like setenv(), but automatically marks the entry as "don't export".
Signed-off-by: Bernhard Nortmann <bernhard.nortm...@web.de>
---
Changes in v2: None
cmd/nvedit.c | 21 +++++++++++++++++++++
include/common.h | 1 +
2 files changed, 22 insertions(+)
Reviewed-by: Simon Glass <s...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
