From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Tue, 8 Dec 2015 12:34:55 -0700 Subject: [U-Boot] [PATCH 4/7] lib: Split panic functions out of vsprintf.c In-Reply-To: <1449559652.2948.3.camel@collabora.co.uk> References: <1449268061-805-1-git-send-email-sjoerd.simons@collabora.co.uk> <1449268061-805-5-git-send-email-sjoerd.simons@collabora.co.uk> <1449559652.2948.3.camel@collabora.co.uk> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Sjoerd, On 8 December 2015 at 00:27, Sjoerd Simons 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