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