Re: [PATCH] support for xz compression format

[Vladimir 'φ-coder/phcoder' Serbinenko]

Szymon Janc wrote:
> Hello,
>
> Attached patch adds support for xz compressed files. Interface is similar to 
> gzio: grub_xzio_open() and grub_xzfile_open(). Files: xzio.h and xzio.c
>
> Decompression algorithm is imported (no source code changes) from 
> http://tukaani.org/xz/embedded.html and is located in lib/xzembed directory
> (files xz_* except xz_wrap.h which is a glue header)
>
> XZ embedded supports only crc32 integration check so if You want to test it
> use  `xz --check=crc32` to compress,  It also supports BCJ filters (not 
> enabled 
> with this patch) so it is possible to get extra few % compression ratio with 
> executables.
>   
How does xz with and without BCJ performs for i386 code? Perhaps we
would want to replace lzma with xz for core.img compression.
> known issues:
> - decoder dictionary size is hardcoded, discovery at runtime would be better
>   (it's on my TODO list)
>   
Can't grub_realloc be used for this?
> - grub_file_seek() will break reading (not a big deal, xz streams are not
>  seekable afterall, if needed should be possible to implement but will cause 
> read performance penatly)
>   
I see no reason to have xzembed.mod separate from xzio.mod. It should be
all in xzio.mod and use common.rmk and not xzembed.rmk.
+ static grub_uint8_t inbuf[XZBUFSIZ];
+ static grub_uint8_t outbuf[XZBUFSIZ];
Will break if 2 files are opened in the same time.
+       grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
No need for this (already done by grub_malloc)
+       if (transparent)
+         return io;
+       else
+         return 0;
You need to discard or pass any pending errors
+   grub_free (xzio);
+   file->device = 0;
+
Why do you set device to 0?
--- grub2-1.98~experimental.20100120/lib/xzembed/xz_crc32.c    
2010-01-20 21:42
:33.000000000 +0100
We already have 2 CRC32 implementations: one in libgcrypt import and one
in crc32.mod. This code duplication is undesirable. I have plans to
remove crc32.mod in favor of libgcrypt implementation. Can you make xz
use libgcrypt import too?
+ #include <grub/gnulib-wrap.h>
I think gnulib-wrap has to be splitted into gnulib-wrap and posix-wrap
+ #define uint8_t grub_uint8_t
+ #define uint16_t grub_uint16_t
+ #define uint32_t grub_uint32_t
+ #define uint64_t grub_uint64_t
typedef is more appropriate
>
> Suggestions and comments are welcome :-)
>
>
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

signature.asc
Description: OpenPGP digital signature