On Mon, 2 Mar 2009, Juergen Keil wrote:
> usr/src/cmd/lofiadm/main.c lines 146 - 156:
>
> A usage function was added between the compress2p variable and the
> gzip_compress function. The only use of compress2p is inside
> gzip_compress(). Move the static variable compress2p inside gzip_compress().
>
>
> usr/src/cmd/lofiadm/main.c lines 186, 192, 206:
>
> Style function definition: move function identifier to column 0
>
> static void *
> SzAlloc(void *p, size_t size)
>
> static void
> SzFree(void *p, void *address, size_t size)
>
> static int
> lzma_compress(void *src, size_t srclen, void *dst,
> size_t *dstlen, int level)
Changed all of the above.
> 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. (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.
> usr/src/uts/common/io/lofi.c lines 199, 205
>
> Style function definition: move function identifier to column 0
>
>
>
> usr/src/uts/common/io/lofi.c line 951
>
> Shouldn't this assert test for ASSERT(i < lsp->ls_comp_index_sz - 1) ?
>
>
> usr/src/uts/common/io/lofi.c line 958
>
> The test for "(i == eblkno)" should be deleted.
> Testing "(i == lsp->ls_comp_index_sz - 2)" should be sufficient.
>
> /*
> * The last segment is special in that it is
> * most likely not going to be the same
> * (uncompressed) size as the other segments.
> */
> if ((i == eblkno) &&
> (i == lsp->ls_comp_index_sz - 2)) {
> seglen = lsp->ls_uncomp_last_seg_sz;
> } else {
> seglen = lsp->ls_uncomp_seg_sz;
> }
I've made all of the above changes and regenerated the
webrev -
http://cr.opensolaris.org/~aalok/lzma-lofi/
Thanks for the review!
Alok