feat(decompression): improved ACL asserts when initializing a decompression_context #505
Conversation
|
|
|
Sorry, only just realised that ACL targets C++11! I'll need to rewrite this to not use |
| ACL_ASSERT(is_valid, "Invalid compressed tracks instance"); | ||
| const error_result error = skip_safety_checks ? error_result() : tracks.is_valid(false); | ||
| const bool is_valid = skip_safety_checks || error.empty(); | ||
| ACL_ASSERT(is_valid, "Invalid compressed tracks instance: %s", error.c_str()); |
There was a problem hiding this comment.
Minor tweak: we can't rely on '%s' as a format specifier because hosts can override the assert macro and might expect a unicode string when %s is present. To support this, we have to use ACL_ASSERT_STRING_FORMAT_SPECIFIER instead
There was a problem hiding this comment.
Thanks @nfrechette! This should be fixed now - I assume there isn't anything similar for %u I need to use?
There was a problem hiding this comment.
Yes, that's correct. Thank you!
nfrechette
changed the title
|
Good stuff! Thank you for the contribution. |
|
Apologies for the slow reply, once CI completes, I'll merge this. |
|
@nfrechette it looks like appveyor might have timed out again? Seems like all the individual builds passed |
|
Yeah, appveyor has been struggling. The builds had been hovering around 50-55mins for some time now, and it looks like a configuration change on their end pushed it over the 1h limit. I'll have to disable some of the slower tests with the older vs2015 toolchain which is fine since it's so ancient now. Thank you for the contribution! |
|
@all-contributors add @Yusuf-PG as code |
|
I've put up a pull request to add @Yusuf-PG! 🎉 |
After running into an issue where ACL was failing to initialize the decompression_context, I noticed that ACL already has the info for why the initialization failed in the
error_result, we just weren't showing it in the assertSmall change to the initialize and relocate functions that adds more useful info to the asserts when possible