On Mon, 2 Mar 2009, Juergen Keil wrote:
> lofiadm/main.c line 157:
>
> The local variable compres2p in function gzip_compress() / lofiadm/main.c
> should remain a static variable, so that openlib() and dlsym() is used
> exactly once.
>
>
> lofiadm/main.c line 184, 185:
>
> The '*' for the void * return type of SzAlloc function should be moved
> up one line
> (I think the idea is that a grep for "^SzAlloc" should find the
> function's definition):
Ah, I see. I'll change now that I know what is the
intent behind this change.
> static void *
> SzAlloc(..)
>
> Same in lofi.c, lines 199, 200
>
>
>>> usr/src/cmd/lofiadm/main.c lines 227 .. 231
>>>
>>> Why is the space available in the destination buffer for compressed data
>>> added to the compressed output? The information seems to be unused.
>>> And this seems to waste 8 bytes for every compressed segment. When
>>> cleaning this, all defines for LZMA_SIZE_OFFSET should be removed from
>>> lofiadm.c, lofi.c; and LZMA_HEADER_SIZE == LZMA_PROPS_SIZE.
>>
>> This is picked directly from the LZMA SDK. The LZMA
>> properties get stored in the first 8 bytes of every
>> compressed segment.
>
> 8 bytes? LZMA_PROPS_SIZE is defined as 5:
>
> % grep LZMA_PROPS_SIZE usr/src/common/lzma/*.h
> usr/src/common/lzma/LzmaDec.h:#define LZMA_PROPS_SIZE 5
> usr/src/common/lzma/LzmaEnc.h:#define LZMA_PROPS_SIZE 5
That should have been 5 bytes for the properties followed
by 8 bytes of uncompressed segment size.
Here is how the LZMA header looks like (from LZMA SDK 4.62) -
LZMA compressed file format
---------------------------
Offset Size Description
0 1 Special LZMA properties (lc,lp, pb in encoded form)
1 4 Dictionary size (little endian)
5 8 Uncompressed size (little endian). -1 means unknown size
13 Compressed data
>> (though admittedly it seems a little
>> odd for LzmaEncode to take a pointer to the compressed
>> data and not the start of the header)
>>
>> I've gotten rid of LZMA_SIZE_OFFSET.
>
> The LZMA properties are a 5 bytes header, followed by an 8 byte
> compressed buffer length, followed by the lzma compressed data.
>
> The 8 byte compressed buffer length between the lzma properties
> and the lzma compressed data confuses me. It was inserted by this
> piece of code it lofiadm/main.c:
>
> dstp = (Byte *)dst;
> t = *dstlen;
> for (i = 0; i < 8; i++, t >>= 8) {
> dstp[LZMA_SIZE_OFFSET + i] = (Byte)t;
> }
>
> Is this length (*dstlen) needed? I don't think we need this 8-byte
> length field; the lzma compressed data should immediately
> follow the 5 byte lzma props data.
I believe that the uncompressed size is used by LZMA during
decompression.
> Ok, changing this now would make new lzma compressed
> files incompatible with the lzma compressed files that you
> can find on opensolaris global ISOs. We would be changing
> the lzma compressed lofi data format.
>
> Btw. the 8-byte length field in the solaris.zlib image on
> osol-0906-108-global-x86.iso are wrong; I would expect
> to see the value 0x000000000001ffff, but I do find the value
> 0x0001ffff0001ffff ?? Since nobody is using this data,
> it doesn't matter...
That's odd, I'm going to pick apart that iso and let you
know what I see.
Alok
