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

Reply via email to