Re: [RFC] LZO support

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 14.09.2011 21:20, Szymon Janc wrote:
> Hello,
>
> I've implemented support for LZO in grub, this includes:
> - import of minilzo library (from [1])
> - support for LZO (de)compression in btrfs (compress=lzo mount option)
> - lzopio - for reading lzop compressed files
>
+  common = lib/minilzo/minilzo.c;
@@ -1650,6 +1653,14 @@
+  common = lib/minilzo/minilzo.c;
Please don't include same file twice in different modules it creates a
symbol conflict. It's ok to have btrfs depend on lzopio if necessary, at
least for now.
+#include "minilzo.h"
I'd prefer <minilzo.h> and an explicit -I on cppflags if necessary.
-      grub_disk_addr_t *outaddr, grub_size_t *outsize,
+      grub_disk_addr_t * outaddr, grub_size_t * outsize,
this is a stylistic regression. (several such occurences)
-      if (desc->data[desc->depth - 1].iter
-         < desc->data[desc->depth - 1].maxiter)
+      if (desc->data[desc->depth - 1].iter <
+         desc->data[desc->depth - 1].maxiter)
Likewise.
+      grub_uint8_t context[lzopio->ccheck_fun->contextsize];
This isn't necessarily well-aligned. Use grub_uint64_t
context[(lzopio->ccheck_fun->contextsize + 7) / 8];
(several occurences)
+  file->size = GRUB_FILE_SIZE_UNKNOWN;
Several components have trouble with such files. Is there any way to
retrieve the size of lzio compressed file?
> - lzopio works ok with grub-fstest (crc, testload), also able to load modules 
>   compressed with lzop, but compressing all modules and doing 
>   grub-install --modules=lzopio fails to boot, still need to investigate that
That because dependencies on hashes are soft so it's not correctly
tracked by grub-install and so you end up in an infinite recursion.

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

signature.asc
Description: OpenPGP digital signature