[U-Boot] [PATCH 4/7] lib: Split panic functions out of vsprintf.c

[Simon Glass]

Hi Sjoerd,

On 8 December 2015 at 00:27, Sjoerd Simons
<sjoerd.simons at collabora.co.uk> wrote:
>
> On Mon, 2015-12-07 at 17:39 -0700, Simon Glass wrote:
> > > diff --git a/lib/Makefile b/lib/Makefile
> > > index 1f1ff6f..ae84833 100644
> > > --- a/lib/Makefile
> > > +++ b/lib/Makefile
> > > @@ -85,13 +85,13 @@ obj-$(CONFIG_LIB_RAND) += rand.o
> > >  ifdef CONFIG_SPL_BUILD
> > >  # SPL U-Boot may use full-printf, tiny-printf or none at all
> > >  ifdef CONFIG_USE_TINY_PRINTF
> > > -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o
> > > +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o
> > >  else
> > > -obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o
> > > +obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o
> > >  endif
> > >  else
> > >  # Main U-Boot always uses the full printf support
> > > -obj-y += vsprintf.o
> > > +obj-y += vsprintf.o panic.o
> > >  endif
> >
> > Why not just add this outside all the ifdef stuff:
> >
> > obj-y += panic.o
>
> Just keeping the old behaviour, that code was not build for SPL builds
> without serial support before. Do you see a benefit of just always
> building it ?

I cannot see a case where you don't build it:

ifdef CONFIG_SPL_BUILD
# SPL U-Boot may use full-printf, tiny-printf or none at all
ifdef CONFIG_USE_TINY_PRINTF
obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o panic.o strto.o
else
obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o panic.o strto.o
endif
else
# Main U-Boot always uses the full printf support
obj-y += vsprintf.o panic.o strto.o
endif


Every case has panic.o and strto.o. What am I missing?

>
> > >  subdir-ccflags-$(CONFIG_CC_OPTIMIZE_LIBS_FOR_SPEED) += -O2
> > > diff --git a/lib/panic.c b/lib/panic.c
> > > new file mode 100644
> > > index 0000000..e2b8b74
> > > --- /dev/null
> > > +++ b/lib/panic.c
> > > @@ -0,0 +1,45 @@
> > > +/*
> > > + *  linux/lib/vsprintf.c
> >
> > nit: can you please drop this line or fix it?
>
> Sure it's pointless anyway
>
> > > + *
> > > + *  Copyright (C) 1991, 1992  Linus Torvalds
> > > + */
> > > +
> > > +/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */
> > > +/*
> > > + * Wirzenius wrote this portably, Torvalds fucked it up :-)
> > > + */
> >
> > Did any of this code actually come from Linux? If not perhaps invent
> > your own copyright?
>
> I actually went back and checked, vsprintf.c was imported
> in de180e6daa529dc78668c99bdf17a9cdd440782d which is a helpful commit
> with the name "Initial revisions".
>
> Most of the code in vsprintf.c is likely to just come from from linux
> afaik (see lib/vsprintf.c in the linux source) especial in the initial
> linux git repository. Unfortunate unless you actually want to go
> trawling back through pre-git linux versions to work out how similar
> the were at the branching point.
>
> The panic functions don't appear in git versions of linux, but may or
> may not be there in pre-git versions.
>
> In this case I just took the simple/conservative path and copied the
> copyright header (assuming correctness which seems reasonable enough)
> when splitting things up. I'd be fine with adjusting the copyright
> header _if_ there was more (easily) available historical data about the
> authors. Given that doesn't seem to be the case, I would prefer to keep
> the copyright as-is unless someone wants to take the time to do some
> code archaeology :)
>
>
> --
> Sjoerd Simons
> Collabora Ltd.

Regards,
Simon