Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maintenance: update vendored minizip code to the version distributed with zlib v1.3 #420

Merged
merged 2 commits into from
Nov 18, 2023
Merged

Maintenance: update vendored minizip code to the version distributed with zlib v1.3 #420

merged 2 commits into from
Nov 18, 2023

Conversation

jayaddison
Copy link
Contributor

Updates the vendored copy of minizip within libxlsxwriter to the version distributed with version 1.3 of zlib with a few libxlsxwriter customizations retained / re-applied.

I have run the tests (make test) and valgrind checks (make test_valgrind) on a system where minizip is not available as a system-provided library (to make sure that the vendored copy of the code was in use) and these passed.

Abbreviated output from the tests is included below:

$ make test
...
RESULTS: 430 tests (430 ok, 0 failed, 0 skipped) ran in 40.4 ms
...
=============================================================== 759 passed in 8.08s ===============================================================

A small number of warnings were emitted by the valgrind checks; mostly string validation errors (length limits exceeded, unacceptable-null values, and placeholder-count mismatches), and it exited with code 0.

Resolves #419.

@jmcnamara
Copy link
Owner

Thanks that is good work. Particularly picking up the local changes to avoid compilation warning on some systems. I must check if v1.3 actually passes -pedantic -ansi now.

A small number of warnings were emitted by the valgrind checks

These are from test cases that are meant to emit warnings. The are harmless in the context of valgrind checks.

Good work overall. I'm happy to merge.

@jmcnamara jmcnamara merged commit 7e8e9c0 into jmcnamara:main Nov 18, 2023
31 of 40 checks passed
@jayaddison
Copy link
Contributor Author

Thank you, @jmcnamara!

@jayaddison jayaddison deleted the maintenance/update-zlib-1.3 branch November 18, 2023 14:44
@jayaddison
Copy link
Contributor Author

@jmcnamara a note that zlib v1.3.1 is now available, including the cherry-picked security fix. I can open a follow-up pull request with an upgrade to that if you like, or let me know if you'd prefer to apply the upgrade yourself.

@jmcnamara
Copy link
Owner

Thanks. A PR would be helpful.

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.

Maintenance suggestion: update vendored copy of minizip library
2 participants