[MarkRose]

Follow-up to #88, from tdulcet's comment there about unchecked allocations that were missed (factor_test.h).

#88 added the checked MALLOC/CALLOC/REALLOC macros and converted the bare allocation sites, but missed two groups, both in Mfactor:

• the 24 calloc() calls in factor_test.h (the Mfactor self-test that runs at every start), and
• the second bit-map array in factor.c (bit_map2, allocated right next to bit_map, which has its own check).

Neither checks the result, so an allocation failure is a NULL dereference (SIGSEGV) instead of the "out of memory" abort with file, line and function that the other sites give. This changes those 25 calloc calls to CALLOC; nothing else.

Testing

I used an LD_PRELOAD shim that makes the Nth calloc of a given size return NULL, and ran Mfactor -m 521 -bmax 20 (nosimd build):

failed allocation	before	after
first calloc in the self-test (factor_test.h:335)	SIGSEGV in mi64_nega (backtrace confirms the NULL q)	ERROR: Function test_fac, at line 335 of file ../src/factor_test.h / calloc of 80 bytes failed - out of memory
second calloc(4255, 8) (bit_map2)	SIGSEGV	ERROR: Function main, at line 1521 of file ../src/factor.c / calloc of 34040 bytes failed - out of memory
first calloc(4255, 8) (bit_map, already checked)	clean abort	clean abort (unchanged)
no failure injected	exit 0	exit 0

The Mfactor build has the same 127 warnings before and after, and the Mlucas build (nosimd) still builds.

What I left alone

I went through every malloc/calloc/realloc in src/ on main. The rest either check the result already (the ptr_prod pattern in the radix files, the inline if(!x) checks, the ASSERTs) or sit in code that is not built: the TEST_TRIALDIV block in factor.c, a commented-out test in util.c, and the CUDA-only code in util.c, as in #88.

Overlap with other open PRs

No open PR makes this change. #372 and #324 convert other calloc calls in factor.c (different lines from these). Seven open PRs touch factor_test.h (#213, #215, #223, #314, #331, #374, #380) and this merges cleanly with each of them one at a time.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Factor code and self-tests now use CALLOC instead of calloc for the described allocations. Allocation arguments and test logic remain unchanged.

Changes: Factor allocation updates

Layer / File(s)	Summary
Replace factor allocation calls src/factor.c, src/factor_test.h	The bit_map2 allocation and self-test scratch-buffer allocations now use CALLOC. Allocation sizes and test logic remain unchanged.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested reviewers: tdulcet

Merge Risk: Minimal · up to 96ad7

The change adds allocation-failure checks without changing the reported allocation sizes or test logic. No merge-blocking issue is identified.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: replacing plain calloc() calls with CALLOC in the allocations that were missed by the earlier #88 change, namely in factor_test.h and the bit_map2 allocation in factor…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: replacing plain calloc() calls with CALLOC in the allocations that were missed by the earlier #88 change, namely in factor_test.h and the bit_map2 allocation in factor.c. The raw summary and PR objectives both describe this same change, which switches 24 calloc() calls in factor_test.h and one allocation in factor.c to CALLOC so that failed allocations are checked. The title is specific and accurate, though it is slightly long at 76 characters and includes an issue reference and file names.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 2 files.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: bfaaff97-66c2-4803-add0-5c6cc5b55242

Commits

Reviewing files that changed from the base of the PR and between 59d85ac and 96ad766.

Files selected for processing: 2

• src/factor.c
• src/factor_test.h

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

[tdulcet]

Thanks for fixing these. It looks good to me.

This should resolve most of the -Wanalyzer-null-dereference warnings from #392.

[ldesnogu]

LGTM.