Skip to content

fix(allocator): drop _POSIX_C_SOURCE from posix_memalign branch - #6906

Merged
nihui merged 3 commits into
Tencent:masterfrom
ktz03:fix/allocator-posix-c-source
Aug 28, 2026
Merged

fix(allocator): drop _POSIX_C_SOURCE from posix_memalign branch#6906
nihui merged 3 commits into
Tencent:masterfrom
ktz03:fix/allocator-posix-c-source

Conversation

@ktz03

@ktz03 ktz03 commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Makes fastMalloc/fastFree use a consistent posix_memalign/free scheme regardless of whether _POSIX_C_SOURCE was defined by an earlier include (e.g. Python.h).

Fixes #6888

Include order (e.g. Python.h) can flip allocation scheme between mat.cpp and inlined Mat::release(), corrupting the heap.

Fixes Tencent#6888
@github-actions github-actions Bot added the core label Aug 16, 2026
@tencent-adm

tencent-adm commented Aug 16, 2026

Copy link
Copy Markdown
Member

CLA assistant check
All committers have signed the CLA.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a9b26a5895

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/allocator.h Outdated
ktz03 added 2 commits August 16, 2026 15:10
Android also defines __unix__; keep API < 17 on memalign. Align
fastMalloc/fastFree conditions (drop include-order-sensitive
_POSIX_C_SOURCE check).

Fixes Tencent#6888
Use the same platform branch as ncnn::fastMalloc/fastFree so host_ptr
from prefer_host_memory paths is not freed with the wrong scheme.

Fixes Tencent#6888
@ktz03

ktz03 commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Addressed Codex review:

  1. Android API < 17: exclude __ANDROID__ from the unix/posix_memalign branch so older Android still uses memalign.
  2. Vulkan helper: update fastMalloc_with_alignment in allocator.cpp to the same platform condition so ncnn::fastFree matches the allocation scheme.

Also normalized allocator.h to LF line endings.

@codecov-commenter

codecov-commenter commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.41%. Comparing base (c189d88) to head (5ddb6b0).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6906      +/-   ##
==========================================
- Coverage   95.44%   95.41%   -0.03%     
==========================================
  Files         960      960              
  Lines      459631   459655      +24     
==========================================
- Hits       438680   438588      -92     
- Misses      20951    21067     +116     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nihui
nihui merged commit 2130e00 into Tencent:master Aug 28, 2026
108 of 110 checks passed
@nihui

nihui commented Aug 28, 2026

Copy link
Copy Markdown
Member

Thanks for your contribution !

@github-actions

Copy link
Copy Markdown

Please enable github action in YOUR FORKED REPO to make code-format workflow work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

4 participants