Conversation
PR Summary
|
There was a problem hiding this comment.
Code Review
This pull request refactors the PNG encoding logic in SpriteUnpacker to use stbi_write_png_to_func with a std::vector instead of stbi_write_png_to_mem with a raw buffer, which improves memory management and idiomatic C++ usage. A review comment suggests adding a safety check for the size parameter in the write callback to ensure robustness against invalid inputs during vector insertion.
| auto write_to_vec = [](void* context, void* data, int size) { | ||
| auto* vec = static_cast<std::vector<unsigned char>*>(context); | ||
| const auto* bytes = static_cast<const unsigned char*>(data); | ||
| vec->insert(vec->end(), bytes, bytes + size); | ||
| }; |
There was a problem hiding this comment.
The stbi_write_png_to_func callback receives size as an int. While it's unlikely to be negative in a successful scenario, adding a check to handle non-positive size values would make the callback more robust and prevent potential undefined behavior from std::vector::insert if an invalid size is ever passed.
auto write_to_vec = [](void* context, void* data, int size) {
if (size <= 0) {
return;
}
auto* vec = static_cast<std::vector<unsigned char>*>(context);
const auto* bytes = static_cast<const unsigned char*>(data);
vec->insert(vec->end(), bytes, bytes + size);
};- Replace stbi_write_png_to_mem with stbi_write_png_to_func in spratunpack_command.cpp to avoid "undeclared identifier" errors in some stb_image_write.h versions. - Reorder CMakeLists.txt to detect dependencies (libarchive, gettext, zopflipng) before defining targets. This fixes "archive.h not found" errors on macOS where headers are in non-standard brew paths. - Remove redundant source file inclusion in executable targets by linking to spratcore library.
…_png_to_func`