Skip to content

fix: Release full-quantize lookup on repeated setup - #905

Closed
carrerasdarren-cell wants to merge 1 commit into
pnggroup:libpng18from
carrerasdarren-cell:agent/free-repeated-quantize-lookup
Closed

fix: Release full-quantize lookup on repeated setup#905
carrerasdarren-cell wants to merge 1 commit into
pnggroup:libpng18from
carrerasdarren-cell:agent/free-repeated-quantize-lookup

Conversation

@carrerasdarren-cell

Copy link
Copy Markdown

Summary

  • release the previous full-quantization lookup table before replacing it
  • clear the member before allocation so cleanup remains safe if allocation fails

png_set_quantize() may be called more than once on the same png_struct, as
clarified while fixing #690. The earlier fix covers quantize_index in the
non-full-quantize branch, but the full-quantize branch still overwrites
palette_lookup on every call. With the default quantization bit widths, each
repeated call leaks 32,768 bytes.

The explicit NULL assignment also follows the allocation-failure cleanup
pattern used for the other png_struct members fixed in a22696b.

Verification

  • A counting-allocator reproducer reports live_bytes=32768 after two calls on
    the unmodified branch and live_bytes=0 with this change.
  • ctest --test-dir build-leak-proof --output-on-failure -j2 passes all 37
    tests.

@jbowler

jbowler commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Same as #904

You are insisting on some etiolated idea of how to write code that does not comport with ANSI-C.

The data must be released when the png_struct (in this case) or png_info (in the other) is free'ed; in other words the data must be released when the app cleans up.

I suggest that submitted the same bug report multiple times is not productive.

@carrerasdarren-cell

Copy link
Copy Markdown
Author

Closing this narrower follow-up to avoid duplicating the repeated-setter premise already under discussion in #904. I will keep any further evidence consolidated there.

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