Re: [PATCH] kbuild: correct size calculation of bzImgae / fix x86boot

[Sam Ravnborg]

On Sun, Dec 20, 2009 at 02:19:41PM +0300, Michael Tokarev wrote:
> Sam Ravnborg wrote:
> > We use ... printf \x ... when calculating the size of the
> > compressed kernel.
> > Unfortunately dash built-in printf does not support this notation
> > resulting in a non-bootable kernel.
> > 
> > Fix this by always using the external version of printf.
> > 
> > The commit that introduced this bug was:
> > 4a2ff67c88211026afcbdbc190c13f705dae1b59: "kbuild: fix bzImage
> > build for x86"
> 
> It's not that simple Sam.

The whole business using shell scripts to append the size seems broken.
How about moving this functionality to mkpiggy where we are
less shell script dependent.

Something like the following.
I have only tested it lightly (vmlinux.bin did not differ
before/after the patch.

It includes the length also in the gzip case - I dunno if that matters.
Also I dunno if ".long" is the same on 32 and 64 bit.

	Sam

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index f8ed065..d39fe2e 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -54,7 +54,8 @@ suffix-$(CONFIG_KERNEL_BZIP2)	:= bz2
 suffix-$(CONFIG_KERNEL_LZMA)	:= lzma
 
 quiet_cmd_mkpiggy = MKPIGGY $@
-      cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false )
+      cmd_mkpiggy = $(obj)/mkpiggy $< $(vmlinux.bin.all-y) > $@ || \
+                   ( rm -f $@ ; false )
 
 targets += piggy.S
 $(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE
diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
index bcbd36c..f4a1a3f 100644
--- a/arch/x86/boot/compressed/mkpiggy.c
+++ b/arch/x86/boot/compressed/mkpiggy.c
@@ -29,6 +29,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <inttypes.h>
+#include <sys/stat.h>
 
 static uint32_t getle32(const void *p)
 {
@@ -38,12 +39,27 @@ static uint32_t getle32(const void *p)
 		((uint32_t)cp[2] << 16) + ((uint32_t)cp[3] << 24);
 }
 
+static file_size(const char *filename)
+{
+	struct stat statbuf;
+	int res;
+
+	res = stat(filename, &statbuf);
+	if (res < 0) {
+		perror(filename);
+		exit(1);
+	}
+	return (size_t)statbuf.st_size;
+}
+
 int main(int argc, char *argv[])
 {
 	uint32_t olen;
 	long ilen;
 	unsigned long offs;
+	size_t size;
 	FILE *f;
+	int i;
 
 	if (argc < 2) {
 		fprintf(stderr, "Usage: %s compressed_file\n", argv[0]);
@@ -67,6 +83,11 @@ int main(int argc, char *argv[])
 	olen = getle32(&olen);
 	fclose(f);
 
+	i = 2;
+	size = 0;
+	while (i < argc)
+		size += file_size(argv[i++]);
+
 	/*
 	 * Now we have the input (compressed) and output (uncompressed)
 	 * sizes, compute the necessary decompression offset...
@@ -91,6 +112,14 @@ int main(int argc, char *argv[])
 	printf(".globl input_data, input_data_end\n");
 	printf("input_data:\n");
 	printf(".incbin \"%s\"\n", argv[1]);
+
+        /*
+         * Bzip2 and LZMA do not include size in file... so we have to fake that;
+         * append the size as a 32-bit littleendian number as gzip does.
+         */
+	if (size > 0)
+		printf(".long %d\n", size);
+
 	printf("input_data_end:\n");
 
 	return 0;
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index cd815ac..10bef1c 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -211,27 +211,14 @@ cmd_gzip = (cat $(filter-out FORCE,$^) | gzip -f -9 > $@) || \
 # Bzip2
 # ---------------------------------------------------------------------------
 
-# Bzip2 and LZMA do not include size in file... so we have to fake that;
-# append the size as a 32-bit littleendian number as gzip does.
-size_append = printf $(shell						\
-dec_size=0;								\
-for F in $1; do								\
-	fsize=$$(stat -c "%s" $$F);					\
-	dec_size=$$(expr $$dec_size + $$fsize);				\
-done;									\
-printf "%08x" $$dec_size |						\
-	sed 's/\(..\)\(..\)\(..\)\(..\)/\\\\x\4\\\\x\3\\\\x\2\\\\x\1/g'	\
-)
-
 quiet_cmd_bzip2 = BZIP2   $@
-cmd_bzip2 = (cat $(filter-out FORCE,$^) | \
-	bzip2 -9 && $(call size_append, $(filter-out FORCE,$^))) > $@ || \
-	(rm -f $@ ; false)
+cmd_bzip2 = cat $(filter-out FORCE,$^) | \
+	bzip2 -9 > $@ || (rm -f $@ ; false)
 
 # Lzma
 # ---------------------------------------------------------------------------
 
 quiet_cmd_lzma = LZMA    $@
-cmd_lzma = (cat $(filter-out FORCE,$^) | \
-	lzma -9 && $(call size_append, $(filter-out FORCE,$^))) > $@ || \
-	(rm -f $@ ; false)
+cmd_lzma = cat $(filter-out FORCE,$^) | \
+	lzma -9 > $@ || (rm -f $@ ; false)
+
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/