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

Reply via email to