2009/2/26 Alok Aggarwal <Alok.Aggarwal at sun.com>:
> I would like to request a code review for LZMA support
> for lofi.
>
> The webrev is here -
>
> http://cr.opensolaris.org/~aalok/lzma-lofi
>
> A brief description of the support being proposed can
> be found here -
>
> http://www.opensolaris.org/os/community/arc/caselog/2008/785/
>
> Could Sanjay and one other person review the changes? I would
> like to get the review by COB Tuesday, March 3rd.
>
> Thanks,
> Alok
>
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)
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.
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;
}
