[BusyBox] Start of bunzip2 cleanup.

Rob Landley rob at landley.net
Sun Oct 5 08:40:29 UTC 2003


In preparation for adding bzip compression support, I've been reading through 
the bzip2 support, and decided cleaning it up would be a good thing.  (If 
nothing else, the gratuitous malloc and free of three different structures 
for every block of compressed data is just evil.)

My original goal was to avoid touching anything in BZ2_bzDecompress() and up, 
and just clean up the wrappers around them (as Glenn already started to do).  
But then I hit the tangle of incestuous pointers between DState, bz_stream, 
and bzFile.

if bzFile goes away and becomes local variables in uncompressStream (which 
it's sort of crying out for), then the strm member of DStream should 
logically be an inline bz_stream structure rather than a pointer to one, 
which also allows the state member of bz_stream to go away since that points 
to the DState it lives in.

Each DState has exactly one bz_stream associated with it, and each bz_stream 
has exactly one DState.  (Each bzFile has one of each, but that's an optional 
wrapper.)  The fact bz_stream and DState are seperately allocated and have 
pointers to each other is a bad sign.

This patch untangles them.  (It's not really intended to reduce code size in 
and of itself, just make further cleanups possible.)  I'd appreciate some 
additional eyeballs confirming I didn't screw up, since I'm basically 
touching high-voltage code here. :)

With this patch, bz_stream becomes the first member of DState (I.E. no longer 
allocated separately), several circular pointer groupings are turned into 
trees, and all the references that need to be adjusted because of this are 
adjusted.

All this is just basically paving the way for further cleanup.  I fixed up 
code in this patch I intend to rip out soon.  But this is the minimal patch 
that needs to touch things in BZ2_bzDecompress()  and above, and does just 
the adjustment there that's necessary and very little else.  (Okay, one 
whitespace cleanup, the removal of a duplicate #define, and adding the vi 
tabstop comment to the top of the file.  But that's it.)

The really annoying part of this is that the argument passed to 
BZ2_bzDecompress() becomes a DState pointer instead of a bz_stream pointer.  
(I could continue to pass the bz_stream pointer and just typecast it, since 
it's the first member, but that's evil.)  Since bz_stream already had to have 
a member intialized to point to a valid DState anyway, this isn't really 
anything more than a cosmetic change.  But it's still annoying, since this is 
part of the library api bzip2 documents.  (Admittedly, in our code it's a 
static function, but still.  It's the principle of the thing.)

This patch compiled and had no errors doing:

./busybox bzcat ../../newfirmware/base/linux-2.6.0-test6.tar.bz2 | tar tv

The rest of the cleanup I currently have planned shouldn't touch anything in 
bzDecompress or above, just in the wrapper around bzDecompress.  (I.E. 
bzReadOpen, bzReadClose, and read_bz2 can all be merged into uncompressStream 
and somewhere between 1/2 and 2/3 of the resulting mess can be just deleted.)

Note: the void * removed from bz_stream was actually a DState pointer, they 
just declared their structures in the wrong order and didn't have the type 
available yet.  I moved bzFile down past DState for a similar reason, 
although the only actual change to bzFile was swapping the DState * for a 
bz_stream pointer (and a name change to force the compiler to complain about 
any code that used it that I didn't see).

The entire rest of the patch is fallout from this.

Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bzip2.patch
Type: text/x-diff
Size: 5681 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20031005/552c1770/attachment.bin 


More information about the busybox mailing list