Re: [RFC] Support for bzip compressed modules

[James Antill <jantill@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3014 bytes --]

On Tue, 2007-01-09 at 10:51 -0500, Karl MacMillan wrote:
> James Antill wrote:
> > On Mon, 2007-01-08 at 15:34 -0500, Karl MacMillan wrote:

> >> I didn't want to add additional set functions - I would have preferred 
> >> to allow sepol_policy_file_set_[mem,fd] to transparently open compressed 
> >> streams with functions to set other behaviors as options stored in 
> >> sepol_policy_file_t structs. This was not possible becuase the current 
> >> set functions do not return errors.
> > 
> >  Do we really care about the memory usage, my instinct would be to drop
> > the FILE specific code and just dump everything into memory and then
> > call the mem_set function and thus. have only one decompression loop
> > (adding the fd version is simple then too).
> >  Calling fstat(fileno(fp)) to read the policy in is probably easier than
> > a loop.
> > 
> 
> Not certain what you are getting at - both code paths result in an 
> uncompressed copy of the compressed data in memory. The only difference 
> is whether we are decompressing from an fd or from another memory buffer.

 Just that it seems easier to have the set_fp() function load all the
data into memory and call the set_mem() function, and have all of the
bz2 stuff in just the set_mem() function.

> > . check is always true in callers, and I'm not sure why you'd have it
> > zero.
> > 
> 
> The magic number checking seems fragile - I'm assuming it might be 
> necessary to force the stream as compressed at some point. Since we are 
> maintaining ABI for this library (and these functions), seems better to 
> be safe.

 Do policy files not have a magic value, then? You can also (for bz2)
check that the next value is between 1 and 9 (it's the compression
ratio).
 I assume this means you'd rely on anything ending in ".bz2" being
compressed, and not otherwise ... do we always have a filename?

> > . All code paths have:
> > 
> >  if (set_foo_bz2() == FAILED)
> >   set_foo();
> > 
> > ...which tells me set_foo_bz2() should do that ... in fact it seems sane
> > to just change set_foo() to check of bz2ness and do the right thing,
> > without having to alter the callers.
> > 
> 
> Note my comments with the original patch - this isn't possible because 
> set_foo() has a void return and we want to maintain binary compatibility.

 Right, but as I said the error paths always just try again without
compression ... so why not just try the compression at the start of the
set_foo() code. You get the same behaviour.

> >  I assume the only reason you went with bzip2 over gzip is the "have to
> > init yourself in the set_mem case"?
> 
> No - just better compression.
> 
> [kmacmill@localhost ~]$ ls -l base.pp.*
> -rw-r--r-- 1 kmacmill kmacmill  86379 Jan  9 10:50 base.pp.bz2
> -rw-r--r-- 1 kmacmill kmacmill 167382 Jan  9 10:50 base.pp.gz

 Wow ... that's better than usual. Do you have the same difference for
smaller modules?

-- 
James Antill <jantill@redhat.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]