Re: [PATCH][UPDATED] support for xz compression format

[Vladimir 'φ-coder/phcoder' Serbinenko]

Szymon Janc wrote:
> Hello,
>
> New version of xz compression patch.
> Changes since then:
> - it works now :-)
> - decoder dictionary can be enlarge up to DICT_BIT_SIZE defined in xz_lzma2.h
>  (currently set to 30 = 128MiB)
> - xz uses crc32 from libgcrypt-grub, internal crc implementation removed
> - removed linux kernel related code from xz
> - get rid of some not needed memcpy() calls
> - xzio.mod stuff made according to Vladimir's comments
> - simplified wraper (only 1 file - xz_wrap.h)
> - number of other improvments and tons of bugfixes
> - files licence changed from public domain to gpl3+
>
> This patch also changes gnulib-wrap.h true/false definitions to make them 
> truly
> constants.
>
> TODO and other questions:
> - lack of file seek support, if files is compressed with small block size, it 
> is 
>   possible to implement pseudo-random access
> - what is optimal i/o buffer size? like BUFSIZ macro in glibc
> - default dictionary size should be chosen for files compression
> - still need to do performance tests
> - introduce some common layer for xzio/gzio etc ?
>
> Comments are welcome
>
>   
First of all: could you use unified diff? (-u option)
  # Misc.
! pkglib_MODULES += gzio.mod bufio.mod elf.mod xzio.mod
Please put pkglib_MODULES += xzio.mod right before other variables

grub2-1.98~experimental.20100120/conf/xzembed.rmk
I don't see a need for either separate .rmk or separate module

+ static grub_uint8_t inbuf[XZBUFSIZ];
+ static grub_uint8_t outbuf[XZBUFSIZ];
Avoid static variables. It will fail if user e.g. mounts xz file as a
loopback then opens xz'ed file on this loopback. Just put buffers in
grub_xzio
+   if (! file)
+   {
+       grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
+     return 0;
grub_error is already issued by grub_malloc, no need to put the same
code here.
+   grub_memset (xzio, 0, sizeof (*xzio));
Use grub_zalloc instead of malloc + memset
+   xzio->buf.out_size = len >= XZBUFSIZ ? XZBUFSIZ : len;
I suggest adding parenthesis around condition
+     /* feed input */
Please capitalise first letter and terminate the comments with a full
stop and 2 spaces.
+         readret = grub_file_read(xzio->file,inbuf,XZBUFSIZ);
Missing spaces before opening bracket and after commas. I recommend
running indent on new files.
> ------------------------------------------------------------------------
>
> _______________________________________________
> 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