Re: [AD] Custom packfiles #2

[Elias Pschernig <elias@xxxxxxxxxx>]

On Mon, 2005-01-03 at 12:19 +1100, Peter Wang wrote: 
> Here's my new attempt.  The vtable now looks like:
> 
> struct PACKFILE_VTABLE
> {
>    AL_METHOD(int, pf_fclose, (void *userdata));
>    AL_METHOD(int, pf_getc, (void *userdata));
>    AL_METHOD(int, pf_ungetc, (int c, void *userdata));
>    AL_METHOD(long, pf_fread, (void *p, long n, void *userdata));
>    AL_METHOD(int, pf_putc, (int c, void *userdata));
>    AL_METHOD(long, pf_fwrite, (AL_CONST void *p, long n, void *userdata));
>    AL_METHOD(int, pf_fseek, (void *userdata, int offset));
>    AL_METHOD(int, pf_feof, (void *userdata));
>    AL_METHOD(int, pf_ferror, (void *userdata));
> };
> 
> I had to add pf_ungetc in there for pack_fgets, so I also exposed the 
> previously internal function:
> 
>     AL_FUNC(int, pack_ungetc, (int c, PACKFILE *f));
> 
> Most of the default packfile implementation is put into a vtable as 
> well.  However, it is still considered a special case.

Hm, ideally (without looking at the code again), it would be no special
case. But well, probably should put that as TODO for 4.4. After moving
packfile chunks and encryption to the compatibility layer, or whatever
else requires the special case.

> 
> The LZSS compression code is now exposed to the user via these functions:
> 
>     AL_FUNC(LZSS_PACK_DATA *, create_lzss_pack_data, (void));
>     AL_FUNC(void, free_lzss_pack_data, (LZSS_PACK_DATA *dat));
>     AL_FUNC(int, lzss_write, (PACKFILE *file, LZSS_PACK_DATA *dat, int 
> size, unsigned char *buf, int last));
> 
>     AL_FUNC(LZSS_UNPACK_DATA *, create_lzss_unpack_data, (void));
>     AL_FUNC(void, free_lzss_unpack_data, (LZSS_UNPACK_DATA *dat));
>     AL_FUNC(int, lzss_read, (PACKFILE *file, LZSS_UNPACK_DATA *dat, int 
> s, unsigned char *buf));
> 
> The older encryption method has been ripped out.[1]  There's a big (and 
> ugly) chunk of _pack_fdopen() which I ripped out as it looked to me like 
> it was only to support the older encryption method.  I'm not totally sure.
> 

Heh. Better to just rip it out, instead of leaving in code nobody
understands or is likely to ever will understand.

> I should also mention that packfile_password() has no effect on "files" 
> opened with packfile_fopen_vtable().

Well, we should document when exactly it has effect, then.

> Chris and Elias, have at it.

Heh. I guess, it really can be left to addons to provide proper open
functions, and therefore they need not be in the vtable.

One other thing I'm thinking is, maybe it would be a good time to split
file.c up? 3000+ lines just is too big for me. E.g. file.c, packfile.c
and lzss.c (and then after 4.2.0, some stuff could move to the
compatibility layer files)? If you agree, I can try to split it after
your patch is applied..

> [1] The newer encryption method is exactly the same as the older method 
> except that it worked at higher level.  If the call stack for 
> pack_getc() looks like this:
> 
>     pack_getc(f)
>     f->vtable->pf_getc(f)
>     normal_getc(f)
>     normal_refill_buffer(f)
>     lzss_read(f, unpack_data, ...)
> 
> then the older encryption method worked in lzss_read(), whereas the 
> newer method works in normal_refill_buffer().  It would be possible to 
> keep compatibility with the older method by copying the password into 
> `unpack_data' structure instead of letting it read from `f' (which is 
> supposed to be opaque to lzss_read() now).  This would complicate 
> _pack_fdopen() -- see the aforementioned big and ugly chunk that I 
> ripped out.

Sounds good. As we said, just should mention that old (<3.9.exact
version number) datafiles may not be loaded with 4.2.0, but only 4.1.17.

Now we just need filter chains or something like that, and we have all
the functionality specified in the A5 documents :)

-- 
Elias Pschernig