kyz
commented
7 years ago Thanks for integrating mspack with Python!
However, this patch goes against a base design feature of libmspack: mspack_system and its filenames are completely opaque to libmspack.
mscab_decompressor::open() takes a "filename" which is passed to mspack_system::open() unmodified. What the "filename" means is completely up to the mspack_system implementation. libmspack cannot and does not assume it's a null-terminated string, that's only true for the default POSIX mspack_system implementation. It does not attempt to copy/move the pointer. That's why it's the caller's duty to maintain ownership of the filename pointer.
The example file test/multifh.c demonstrates the use of "filenames" that are actually m_filename structures. If libmspack took on this patch, that example program, and other people's use of the library relying on this design, would immediately break.
SillyBits
…ing a copy which is used instead of pointer passed. Will be cleaned in cabd_close()
While trying to wrap libmspack using pymspack/Cython, I've ran into odd behavior while extracting cabinet contents. Every call on extraction ended up in MSPACK_ERR_OPEN returned which didn't make any sense to me as cabd_open() itself wasn't a problem at all and instance was alive still.
After some debugging, I've discovered the filename passed to cabd_open() got lost and further calls to sys->open within cabd_extract() will use some gargabe as its filename. So I've added some alloc/free to filename handling and callee doesn't need to bother with keeping the string alive until cab gets closed.
With those lines added, pymspack is happily extracting files, no more MSPACK_ERR_OPEN ... heading back to original project where this neat lib is being used. Cheers :-)