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

[Vladimir 'φ-coder/phcoder' Serbinenko]

Szymon Janc wrote:
> On Tue, 16 Feb 2010 14:12:04 +0100  Vladimir 'φ-coder/phcoder' Serbinenko 
> <address@hidden> wrote
>
>   
>> First of all: could you use unified diff? (-u option)
>>     
>
> Sure, will use that in future.
>
>   
>> 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.
>>     
>
> I think You have reviewed old patch, not the updated one :-)
>
>   
Right. Many comments still apply though. Reviewing new one (in addition
to old comments):
+ #define VLI_MAX_DIGITS 9
Are you sure it's 9? It gives only 63 and not 64 bits
+   if (! test_header(file) || ! test_footer(file))
+   {
Seeking to the end of file is very expensive on pxe. Can it be skipped?
grub_xzio_read seems to ignore file->offset. Have you tested seeking?

> ------------------------------------------------------------------------
>
> _______________________________________________
> 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