[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] support for xz compression format
From: |
Vladimir 'φ-coder/phcoder' Serbinenko |
Subject: |
Re: [PATCH] support for xz compression format |
Date: |
Wed, 27 Jan 2010 00:25:54 +0100 |
User-agent: |
Mozilla-Thunderbird 2.0.0.22 (X11/20091109) |
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