[U-Boot] [PATCH] net: Always build the string_to_enetaddr() helper

[Ondřej Jirman]

Hello, 

On Tue, Sep 17, 2019 at 09:22:20PM +0000, Joe Hershberger wrote:
> Hi Simon,
> 
> On Sat, Sep 14, 2019 at 1:55 PM Simon Goldschmidt
> <simon.k.r.goldschmidt at gmail.com> wrote:
> >
> > Joe Hershberger <joe.hershberger at ni.com> schrieb am Sa., 14. Sep. 2019,
> > 20:46:
> >
> > > On Sat, Sep 14, 2019 at 1:32 PM Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Sat, Sep 14, 2019 at 04:05:44PM +0200, Ondřej Jirman wrote:
> > > > > Hi,
> > > > >
> > > > > On Fri, Sep 13, 2019 at 07:40:22PM -0500, Joe Hershberger wrote:
> > > > > > Part of the env cleanup moved this out of the environment code and
> > > into
> > > > > > the net code. However, this helper is sometimes needed even when the
> > > net
> > > > > > stack isn't included.
> > > > > >
> > > > > > Move the helper to lib/net_utils.c like it's similarly-purposed
> > > > > > string_to_ip(). Also rename the moved function to similar naming.
> > > > > >
> > > > > > Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
> > > > > > Reported-by: Ondrej Jirman <megous at megous.com>
> > > > >
> > > > > I've tested the patch and it works, but I'be found other related
> > > issue, where
> > > > > u-boot thinks %pM will format a MAC address string, but it does just
> > > > > print out the pointer due to relevant functions being gated by
> > > CONFIG_CMD_NET
> > > > > guard in lib/vsprintf.c.
> > > > >
> > > > > The gating should probably be done so that it panics/halts the u-boot
> > > if gated
> > > > > pointer flags are used by u-boot code, because that will clearly be
> > > incorrect,
> > > > > without calling code ever knowing. This way the user will know that
> > > something
> > > > > is wrong and will have to fix the code.
> > > >
> > > > I'm not in favor of panic because of calling an unimplemented print
> > > > format character.  I guess we'll need to see what the size increase is
> > > > on un-guarding these formats and go from there.
> > >
> > > I'll look into it. I'm also not in favor of a panic.
> > >
> >
> > In lwIP, we're using macros for such format characters. Would it work to do
> > that here and make the compiler complain about an undefined symbol of the
> > macro for this extended format character isn't defined?
> 
> Maybe... Though, if we don't successfully police the usage of the
> macro, it won't help. I'd like to evaluate the code-size impact and
> maybe just always include it.

How about a one-time console warning if unsupported format modifier is used
at run-time, if panic is too aggressive?

Anything that will hint at the error, would be nice.

regards,
	o.

> -Joe
> 
> > Regards,
> > Simon
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot