search

atomicobject / heatshrink

data compression library for embedded/real-time systems
ISC License
1.33k stars 180 forks source link

heatshrink binary only decompresses the first block #9

Closed philpem closed 9 years ago

philpem commented 9 years ago

After patching Heatshrink with the changes in #7 and building with MinGW (gcc 4.7.2), a valid "heatshrink.exe" executable is produced, but this binary fails to decompress data correctly.

$ ./heatshrink.exe -ev LICENSE LICENSE.enc

$ ./heatshrink.exe -dv LICENSE.enc LICENSE.dec

$ md5sum LICENSE*
cd12e61f206f9ee1a6622d0df2f773d1 *LICENSE
e7e84c0c9268751994a1830f9b98d399 *LICENSE.dec
81239e24ddf1a6dde9d5cff95d186911 *LICENSE.enc

$ ls -l LICENSE*
-rw-r--r-- 1 ppemberton Administrators 784 Dec 12 15:11 LICENSE
-rw-r--r-- 1 ppemberton Administrators 387 Dec 22 11:41 LICENSE.dec
-rw-r--r-- 1 ppemberton Administrators 681 Dec 22 11:41 LICENSE.enc

It appears that the decode step is only processing the first block of data in the file.

Interestingly the byte counts in verbose mode are wrong too:

$ ./heatshrink.exe -ev LICENSE LICENSE.enc
LICENSE 14.29 %  784 -> 672 (-w 11 -l 4)

$ ./heatshrink.exe -dv LICENSE.enc LICENSE.dec
LICENSE.enc -3.27 %      367 -> 379 (-w 11 -l 4)
philpem commented 9 years ago

On quick inspection, there's a marked difference between the encode() and decode() functions in heatshrink.c. It looks like reaching the end of the file might cause the decode() function to exit without properly flushing the decompression buffer (sink_read).

philpem commented 9 years ago

Adding O_BINARY to the open() parameters in handle_open() fixes this nicely, per sjaeckel's PR #4 (commit e9c766b01876100e6829f44556da3f939f99218b).

An easy way to deal with O_BINARY not being defined on Linux would be to #ifdef it at the top of heatshrink.c, and define it as zero if it isn't defined (the platform doesn't differentiate between binary and text files). ORing something with zero obviously has no effect. That saves a bunch of ifdefs and the creation of macros.

If I were to clean up sjaeckel's fixes to apply to current master/tip and submit a PR, would this likely be accepted?

silentbicycle commented 9 years ago

Yes. I may get to it myself today first, though. I'm pretty sure I know what needs to be done to fix it, and will have access to a MinGW environment to test it.

It definitely has to do with binary/non-binary file IO on Windows.

silentbicycle commented 9 years ago

I didn't get to it today and am likely to be busy with things through the holidays, but if you make the necessary changes I'd be happy to merge them. Otherwise I'll get to it soon, this has sat open for too long. :|