Skip to content

Fix memory leak in tryopen. - #592

Open
DeNdlerX wants to merge 1 commit into
nih-at:mainfrom
DeNdlerX:fix-tryopen-leak
Open

DeNdlerX wants to merge 1 commit into
nih-at:mainfrom
DeNdlerX:fix-tryopen-leak

Conversation

@DeNdlerX

Copy link
Copy Markdown

tryopen -s gets the error message with zip_error_strerror() but never calls zip_error_fini(), so the string leaks for every archive that fails to open. When the tests are built with ASan on Linux, this makes open_incons, open_incons_eocd64_gap, open_incons_utf8_surrogate and extra_field_align fail with a LeakSanitizer report.

cmake -B build -DCMAKE_C_FLAGS="-fsanitize=address -g"
cmake --build build
ctest --test-dir build -R 'open_incons|extra_field_align'

On main, 4 of the 5 selected tests fail; with this change all of them pass.

The two other LeakSanitizer failures in the suite (read_incons and decrypt-wrong-password-pkware-2) come from zip_source_free() and are fixed by #591. With both applied, the full test suite passes under ASan with no leaks reported.

If you're interested, I can also add a CI job that runs the tests with -fsanitize=address,undefined on Linux, so issues like these are caught automatically.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants