Calling decompress on a CompressedDataset crashes

[kmorel]

It no longer works to call mgard::decompress on a CompressedDataset<T,Real> (that comes from mgard::compress). Typically you get a crash.

What think is happening is that mgard::compress now puts in a self-describing header. This header is ignored by the overload of decompress that takes a CompressedDataset (even though its buffer has that header). Consequently, parts of the header (like the signature and version numbers) are read as buffer sizes, and things go downhill from there.

[qliu21]

@kmorel Thanks for checking out the new version. For decompression, the following API needs to be called.

std::unique_ptr<unsigned char const []> decompress( void const *const compressed_buffer, std::size_t compressed_size) {

[ben-e-whitney]

Your diagnosis is right, @kmorel. You shouldn't have to call mgard::decompress(void const * const, const std::size_t) now – calling mgard::compress followed by mgard::decompress(const CompressedDataset &) should still work. I assumed it still did because the tests in tests/src/test_compress.hpp still passed. However, in 6d138ed4a163195e334f42a327a997fbae41e9c8 the decompression call in those tests was changed to the new overload, so the regression wasn't caught. @qliu21, you should have added new tests rather than repurposing the existing ones. I will fix this after finishing the new serialization implementation.

[qliu21]

Ben,

I am not sure if I follow this. If I didn't repurpose the existing tests, they will all fail. On the other hand, the new API is something that we all agreed on. It looks like you really want the old API to be resuscitated. Am I missing something?

G

On Wed, Oct 20, 2021 at 4:26 PM ben-e-whitney @.***> wrote:

Your diagnosis is right, @kmorel https://github.com/kmorel. You shouldn't have to call mgard::decompress(void const * const, const std::size_t) now – calling mgard::compress followed by mgard::decompress(const CompressedDataset &) should still work. I assumed it still did because the tests in tests/src/test_compress.hpp still passed. However, in 6d138ed https://github.com/CODARcode/MGARD/commit/6d138ed4a163195e334f42a327a997fbae41e9c8 the decompression call in those tests was changed to the new overload, so the regression wasn't caught. @qliu21 https://github.com/qliu21, you should have added new tests rather than repurposing the existing ones. I will fix this after finishing the new serialization implementation.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CODARcode/MGARD/issues/153#issuecomment-948010351, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYYULDJ7VYGC2O2YT53LZTUH4QW7ANCNFSM5FPR4KVQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Dr. Qing Liu Assistant Professor Helen and John C. Hartmann Department of Electrical and Computer Engineering New Jersey Institute of Technology, Newark, NJ Web: https://web.njit.edu/~qliu/ Email: @.*** Phone: 973-596-3526

[ben-e-whitney]

I think there was a miscommunication about the old API. I don't remember there ever being a discussion about removing it. In any case, we never did remove it, and now that we've released version 1.0.0 we'll need to keep it working till at least version 2.0.0 so that we don't break backwards compatibility. I will reinstate the tests and get them working after I finish the new header development. It won't be too hard.