Messages in this thread | | | Date | Thu, 11 Sep 2008 15:43:53 -0700 | From | "H. Peter Anvin" <> | Subject | Re: [update3] [PATCH] init: bzip2 or lzma -compressed kernels and initrds |
| |
Alain Knaff wrote: > From: Alain Knaff <alain@knaff.lu> > > This is an updated version of my bzip2/lzma patch > > It is based on an idea by Christian Ludwig, includes support for > compressing the kernel with bzip2 or lzma rather than gzip. Both > compressors give smaller sizes than gzip. Lzma's decompresses faster > than bzip2. > > It also supports ramdisks and initramfs' compressed using these two > compressors. > > The functionality has been successfully used for a couple of years by > the udpcast project > > This version applies to "tip" kernel 2.6.27-rc5 > > Changes since last version (update2): > > - Removed NEW_CODE #ifdef : non-Intel architectures will be patched > eventually anyways > - Replaced IN_MEMORY #ifdefs by (less efficient) runtime checks > > Signed-off-by: Alain Knaff <alain@knaff.lu>
Looks a lot better now. See the following, though:
> -#define OF(args) args > #define STATIC static > +#define NEW_CODE
Should be deleted, right?
> > - phdrs = malloc(sizeof(*phdrs) * ehdr.e_phnum); > + phdrs = (void *) malloc(sizeof(*phdrs) * ehdr.e_phnum); > if (!phdrs)
malloc() returns a void pointer already...
> + > +#ifdef CONFIG_RD_BZIP2 > + /* > + * If it matches the bzip magic numbers, return -1 > + */ > + if (buf[0] == 0x42 && (buf[1] == 0x5a)) { > + printk(KERN_NOTICE > + "RAMDISK: Bzipped image found at block %d\n", > + start_block); > + *uncompressor = bunzip2; > nblocks = 0; > goto done; > } > +#endif > + > +#ifdef CONFIG_RD_LZMA > + /* > + * If it matches the bzip magic numbers, return -1 ^^^^ ???? > + */ > + if (buf[0] == 0x5d && (buf[1] == 0x00)) { > + printk(KERN_NOTICE > + "RAMDISK: Lzma image found at block %d\n", > + start_block);
> -#define OF(args) args > +#ifdef CONFIG_RD_BZIP2 > +#include <linux/decompress_bunzip2.h> > +#endif > +#ifdef CONFIG_RD_LZMA > +#include <linux/decompress_unlzma.h> > #endif
There is no reason to include header files conditionally.
It might be worthwhile to create a proper subdirectory rather than using an underscored namespace (also, in general, dashes are preferred in filenames.)
> +#ifndef always_inline > +# if defined(__GNUC__) && (__GNUC__ > 3 || __GNUC__ == 3 && __GNUC_MINOR__ > 0) > +# define always_inline __attribute__((always_inline)) inline > +# else > +# define always_inline inline > +# endif > +#endif
Any reason to not just use <linux/compiler.h> here?
> +#ifdef CONFIG_FEATURE_LZMA_FAST > +# define speed_inline always_inline > +#else > +# define speed_inline > +#endif
Should this be always_inline or just plain inline?
-hpa
| |