fix deallocate arguments in monotonic_resource and string_impl - #1179
fix deallocate arguments in monotonic_resource and string_impl#1179Ramya-9353 wants to merge 1 commit into
Conversation
|
An automated preview of the documentation is available at https://1179.json.prtest2.cppalliance.org/libs/json/doc/html/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-08-05 16:56:48 UTC |
|
GCOVR code coverage report https://1179.json.prtest2.cppalliance.org/gcovr/index.html Build time: 2026-08-05 17:05:59 UTC |
|
|
grisumbras
left a comment
grisumbras
left a comment
There was a problem hiding this comment.
Thank you for catching these serious issues.
| sp->deallocate(k_.s, k_.n + 1); | ||
| sp->deallocate(k_.s, k_.n + 1, | ||
| alignof(char)); | ||
| } |
There was a problem hiding this comment.
While this is a correct fix, it is unrelated to the topic of this commit. Please move to a separate PR.
Btw, you don't have to add a line here, the extra argument perfectly fits into 80 columns, which is what I use as the threshold.
There was a problem hiding this comment.
Done, moved it to #1181 and kept the call on one line there.
| { | ||
| checking_resource res; | ||
| monotonic_resource mr(1024, &res); | ||
| mr.allocate(1000, alignof(core::max_align_t)); |
There was a problem hiding this comment.
https://drone.cpp.al/boostorg/json/2138/1/2
Silence these warnings with a cast to void (e.g. (void)mr.allocate...
There was a problem hiding this comment.
Done, cast to void.
| mr.allocate(4000, alignof(core::max_align_t)); | ||
| } | ||
|
|
||
| { |
There was a problem hiding this comment.
This test does not appear to be necessary.
There was a problem hiding this comment.
Dropped it, the direct allocate calls already cover the block bookkeeping.
| } | ||
|
|
||
| void | ||
| testAllocation() |
There was a problem hiding this comment.
I presume, this should also be a part of a separate PR centered on requesting correct alignment in string deallocation.
There was a problem hiding this comment.
Yes, moved to #1181 along with the alignment fix.
016805c to
fca95f2
Compare
|
Trimmed this down to just the monotonic_resource size fix, with the warnings silenced and the parse-through-monotonic test dropped. The key-string alignment fix and its test are now #1181. |
|
|
3 participants
Repro: parse anything through a
monotonic_resourcewhose upstream istest/checking_resource.hpp. Every block failssize == it->second.first.Cause:
do_allocateasks the upstream forsizeof(block) + next_size_but records onlynext_size_inblock::size, sorelease()returns each block 32 bytes short on LP64. The default resource ignores the size argument, so this only bites upstreams that bucket by size.Fix: add the header size back before deallocating. The key-string alignment fix that was originally part of this PR moved to #1181.