Skip to content

libbz2-rs-sys: pass byte counts to custom allocators as (n, 1), like libbzip2 - #149

Open
sergiodj wants to merge 1 commit into
trifectatechfoundation:mainfrom
sergiodj:adjust-bzalloc-count-size
Open

sergiodj wants to merge 1 commit into
trifectatechfoundation:mainfrom
sergiodj:adjust-bzalloc-count-size

Conversation

@sergiodj

@sergiodj sergiodj commented Nov 4, 2026

Copy link
Copy Markdown

libbzip2 requests every block through BZALLOC(nnn), which calls bzalloc(opaque, nnn, 1). libbz2-rs-sys calls bzalloc(opaque, count, size_of::<T>()) instead, so an allocator written against the C library -- one that honours only the second argument, which the C library's calls have always allowed -- returned count bytes for a block of count * size_of::<T>() bytes.

This patch passes the byte count (i.e., items * size) as items and size as 1. A count that does not fit the C int fails like a NULL return.

Disclaimer: an LLM model was used to verify the patch's correctness. It also generated a reproducer for the issue which I'll be happy to post if necessary.

…libbzip2

libbzip2 requests every block through `BZALLOC(nnn)`, which calls
`bzalloc(opaque, nnn, 1)`. libbz2-rs-sys calls `bzalloc(opaque, count,
size_of::<T>())` instead, so an allocator written against the C library
-- one that honours only the second argument, which the C library's
calls have always allowed -- returned `count` bytes for a block of
`count * size_of::<T>()` bytes.

This patch passes the byte count (i.e., `items * size`) as `items` and
size as 1. A count that does not fit the C int fails like a NULL
return.

Signed-off-by: Sergio Durigan Junior <sergiodj@chainguard.dev>
@folkertdev

Copy link
Copy Markdown
Member

one that honours only the second argument, which the C library's calls have always allowed

What do you mean by this? bzalloc cannot just return the third argument?! That would be overfitting to the implementation, and is IMO not a behavior we need to preserve.

@sergiodj

sergiodj commented Nov 4, 2026

Copy link
Copy Markdown
Author

Thanks for the quick review.

That's a fair point. My point here is that libbzip2 seems to have always called hooks as bzalloc(opaque, n, 1), even though their manual states:

Custom memory allocators are supported, via fields bzalloc, bzfree, and opaque. The value opaque is passed to as the first argument to all calls to bzalloc and bzfree, but is otherwise ignored by the library. The call bzalloc ( opaque, n, m ) is expected to return a pointer p to n * m bytes of memory, and bzfree ( opaque, p ) should free that memory.

If libbzip2-rs calls bzalloc(opaque, n * m, 1) then all user-provided allocators that just ignore the third argument will misbehave with it, while still working with libbzip2. OTOH I understand your point of not wanting to replicate poor decisions here, and I totally get if you just reject the PR.

How would you feel about a patch to the documentation explaining that user-supplied hooks must honour both arguments?

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