[PATCH v2 04/13] env: Change env_match() to static and remove from header

[Simon Glass]

Hi Marek,

On Thu, 14 Oct 2021 at 10:06, Marek Behún <kabel at kernel.org> wrote:
>
> On Thu, 14 Oct 2021 09:11:08 -0600
> Simon Glass <sjg at chromium.org> wrote:
>
> > Hi Marek,
> >
> > On Wed, 13 Oct 2021 at 09:46, Marek Behún <kabel at kernel.org> wrote:
> > >
> > > From: Marek Behún <marek.behun at nic.cz>
> > >
> > > This function was used by other parts of U-Boot in the past when
> > > environment was read from underlying device one character at a time.
> > >
> > > This is not the case anymore.
> > >
> > > Signed-off-by: Marek Behún <marek.behun at nic.cz>
> > > ---
> > >  cmd/nvedit.c  | 30 +++++++++++++++---------------
> > >  include/env.h | 11 -----------
> > >  2 files changed, 15 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> > > index ddc715b4f9..742e0924af 100644
> > > --- a/cmd/nvedit.c
> > > +++ b/cmd/nvedit.c
> > > @@ -706,6 +706,21 @@ char *from_env(const char *envvar)
> > >         return ret;
> > >  }
> > >
> >
> > Please can you add the function comment here? We don't want to lose
> > it.
>
> Simon, the comment is invalid (the function does something
> different from what the comment says) and the function is only used as a
> helper by env_get_f(), which comes right after it. The function is
> refactored and renamend in subsequent patches, and its purpose seems
> obvious to me.
>
> Should I really leave the comment there?

That's fine but it would be good to mention that in the commit message
explicitly.

Also you add matching_name_get_value(). Can you add a comment to that, then?

Regards,
Simon