fix: handle zero secure allocator pool count

[thepastaclaw]

fix: handle zero secure allocator pool count

Issue being fixed or feature implemented

std::thread::hardware_concurrency() is allowed to return 0.
mt_pooled_secure_allocator used that value directly as the secure pool count,
which could leave the allocator with no pools and make the first allocation
compute a bucket modulo zero.

This keeps normal behavior unchanged while making the edge case safe.

What was done?

• Added a fallback so a zero secure allocator pool count is treated as one pool.
• Added a focused allocator unit test that forces a zero pool count and verifies
  allocation/deallocation still work.

How Has This Been Tested?

Tested on macOS arm64 with the depends config site build environment.

• git diff --check
• make -C src -j8 test/test_dash
• Focused unit test:
  allocator_tests/mt_pooled_secure_allocator_zero_pool_count
• Full allocator unit suite: allocator_tests
• Pre-PR code review gate: ship

Breaking Changes

None.

Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone
  (for repository code-owners and collaborators only)

[thepastaclaw]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fb3ae1ba-7489-4b08-a208-bbadb2d86646

📥 Commits

Reviewing files that changed from the base of the PR and between 317917a and 427a5af.

📒 Files selected for processing (2)

• src/support/allocators/mt_pooled_secure.h
• src/test/allocator_tests.cpp

---

Walkthrough

This PR makes the pools_count parameter of mt_pooled_secure_allocator explicit and configurable in the constructor, rather than always computing it internally. The constructor now accepts an optional pools_count argument defaulting to std::thread::hardware_concurrency() and guards against zero by clamping it to 1. A new test validates that the allocator behaves correctly when instantiated with zero pools.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: handling the edge case of zero pool count in the secure allocator.
Description check	✅ Passed	The description comprehensively explains the issue, solution, testing approach, and includes a detailed checklist related to the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Infer (1.2.0)

src/test/allocator_tests.cpp

src/test/allocator_tests.cpp:5:10: fatal error: 'support/allocators/mt_pooled_secure.h' file not found
5 | #include <support/allocators/mt_pooled_secure.h>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
Error: the following clang command did not run successfully:
/opt/infer-linux-x86_64-v1.2.0/lib/infer/facebook-clang-plugins/clang/install/bin/clang-18
@/tmp/coderabbit-infer/427a5af911a506b4e8e32bbeb3e83913ed6ce0c3-46b73fc223f632ff/tmp/clang_command_.tmp.9e518b.txt
++Contents of '/tmp/coderabbit-infer/427a5af911a506b4e8e32bbeb3e83913ed6ce0c3-46b73fc223f632ff/tmp/clang_command_.tmp.9e518b.txt':
"-cc1" "-load"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../../facebook-clang-plugins/libtooling/build/FacebookClangPlugin.dylib"
"-add-plugin" "BiniouASTExporter" "-plugin-arg-BiniouASTExporter" "-"
"-plugin-arg-BiniouASTExporter" "PREPEND_CURRENT_DIR=1"
"-plugin-arg-BiniouASTExporter" "MAX_STRING_SIZE=65535" "-cc1" "-tripl

... [truncated 1148 characters] ...

nstall/lib/clang/18/include"
"-internal-isystem" "/usr/local/include" "-internal-isystem"
"/usr/lib/gcc/x86_64-linux-gnu/12/../../../../x86_64-linux-gnu/include"
"-internal-externc-isystem" "/usr/include/x86_64-linux-gnu"
"-internal-externc-isystem" "/include" "-internal-externc-isystem"
"/usr/include" "-Wno-ignored-optimization-argument" "-Wno-everything"
"-fdeprecated-macro" "-ferror-limit" "19" "-fgnuc-version=4.2.1"
"-fskip-odr-check-in-gmf" "-fcxx-exceptions" "-fexceptions"
"-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o"
"/tmp/coderabbit-infer/46b73fc223f632ff/file.o" "-x" "c++"
"src/test/allocator_tests.cpp" "-O0" "-fno-builtin" "-include"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../lib/clang_wrappers/global_defines.h"
"-Wno-everything"

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[thepastaclaw]

✅ Review complete (commit 427a5af)

[thepastaclaw]

Code Review

Both agents independently flagged no in-scope issues for this small, focused fix. The change correctly clamps pools_count to a minimum of 1 to prevent modulo-by-zero in get_bucket(), and the added unit test deterministically exercises the zero-pool path via the new explicit constructor parameter.

Note: GitHub does not allow approving the author's own PR, so this clean automated review is posted as a neutral comment.

[PastaPastaPasta]

utACK 427a5af