[4/7] lib: Split panic functions out of vsprintf.c

Message ID 1449268061-805-5-git-send-email-sjoerd.simons@collabora.co.uk
State New
Headers show

Commit Message

Sjoerd Simons Dec. 4, 2015, 10:27 p.m.
To allow panic and panic_str to still be used when using tiny-printf,
split them out into their own file which gets build regardless of what
printf implementation is used.

Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>

---

 lib/Makefile   |  6 +++---
 lib/panic.c    | 45 +++++++++++++++++++++++++++++++++++++++++++++
 lib/vsprintf.c | 29 -----------------------------
 3 files changed, 48 insertions(+), 32 deletions(-)
 create mode 100644 lib/panic.c

-- 
2.6.2

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Sjoerd Simons Dec. 8, 2015, 7:27 a.m. | #1
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 ?

> >  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 Dec. 14, 2015, 7:38 a.m. | #2
On Sun, 2015-12-13 at 20:45 -0700, Simon Glass wrote:
> On 8 December 2015 at 12:38, Simon Glass <sjg@chromium.org> wrote:
> > Hi Scott,
> > 
> > On 8 December 2015 at 12:36, Scott Wood <scottwood@freescale.com>
> > wrote:
> > > On Tue, 2015-12-08 at 12:34 -0700, Simon Glass wrote:
> > > > Hi Sjoerd,
> > > > 
> > OK, so how about this:
> > 
> > > > ifdef CONFIG_SPL_BUILD
> > > > # SPL U-Boot may use full-printf, tiny-printf or none at all
> > 
> > obj-$(CONFIG_SPL_SERIAL_SUPPORT) += panic.o strto.o
> > 
> > > > ifdef CONFIG_USE_TINY_PRINTF
> > obj-$(CONFIG_SPL_SERIAL_SUPPORT) += tiny-printf.o
> > > > else
> > obj-$(CONFIG_SPL_SERIAL_SUPPORT) += vsprintf.o
> > > > endif
> > > > else
> > > > # Main U-Boot always uses the full printf support
> > > > obj-y += vsprintf.o panic.o strto.o
> > > > endif
> 
> It's just a nit so I'm going to leave it as is for now.
> 
> Applied to u-boot-rockchip, thanks!

Heh, i just did an update series last night to send this morning :)
lovely timing.

I'll drop you some patches for these nits later this week while i still
remember them :) (I agree your suggestion here is nicer).

Patch

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
 
 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
+ *
+ *  Copyright (C) 1991, 1992  Linus Torvalds
+ */
+
+/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */
+/*
+ * Wirzenius wrote this portably, Torvalds fucked it up :-)
+ */
+
+#include <common.h>
+#if !defined(CONFIG_PANIC_HANG)
+#include <command.h>
+#endif
+
+static void panic_finish(void) __attribute__ ((noreturn));
+
+static void panic_finish(void)
+{
+	putc('\n');
+#if defined(CONFIG_PANIC_HANG)
+	hang();
+#else
+	udelay(100000);	/* allow messages to go out */
+	do_reset(NULL, 0, 0, NULL);
+#endif
+	while (1)
+		;
+}
+
+void panic_str(const char *str)
+{
+	puts(str);
+	panic_finish();
+}
+
+void panic(const char *fmt, ...)
+{
+	va_list args;
+	va_start(args, fmt);
+	vprintf(fmt, args);
+	va_end(args);
+	panic_finish();
+}
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index dd8380b..bf5fd01 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -897,35 +897,6 @@  int vprintf(const char *fmt, va_list args)
 	return i;
 }
 
-static void panic_finish(void) __attribute__ ((noreturn));
-
-static void panic_finish(void)
-{
-	putc('\n');
-#if defined(CONFIG_PANIC_HANG)
-	hang();
-#else
-	udelay(100000);	/* allow messages to go out */
-	do_reset(NULL, 0, 0, NULL);
-#endif
-	while (1)
-		;
-}
-
-void panic_str(const char *str)
-{
-	puts(str);
-	panic_finish();
-}
-
-void panic(const char *fmt, ...)
-{
-	va_list args;
-	va_start(args, fmt);
-	vprintf(fmt, args);
-	va_end(args);
-	panic_finish();
-}
 
 void __assert_fail(const char *assertion, const char *file, unsigned line,
 		   const char *function)