An ffi_func's :binstr or :string argument takes a String as :str does

[y-yagi]

What this changes

:binstr and the ffi gem's :string are const char * like :str, but the argument lowering and the count of String arguments compared the spec to "str" alone, so both fell through to the integer path. A String-typed value still worked, by accident; a boxed one (a mixed Array's element, a mixed Hash's value, a poly local) raised no implicit conversion of String into Integer, nil raised instead of passing NULL, and a boxed Integer went to C as a pointer. An ffi_callback trampoline did the same for a poly parameter: the method got the pointer as an Integer and NULL as 0.

All three places ask ffi_spec_is_str now, which the String rooting beside them already did. The return side is unchanged: a :str return is read up to the first NUL, a :binstr return is sp_ffi_bin_len bytes. FFI.md says so instead of calling :binstr return-only.

test/ffi_str_spec_aliases_arg.rb covers both specs as an argument (typed, nil, mixed Array and Hash element, poly local, a boxed Integer, two in one call) and as a callback parameter; before, it stops at the first nil.

make gate (on this branch merged with current master)

progressing

paste the Tests:, scale-test and gate: lines here

• New tests have .expected files that match CRuby 4.0 run with --enable-frozen-string-literal
• Values past 2^31 are marked # spinel: int64
• If optcarrot's generated C changed: callgrind numbers, checksum 59662
• Depends on: #

Summary by CodeRabbit

• New Features
  • FFI string specifications now support :binstr and :string alongside :str for call arguments and callback parameters. nil is passed as NULL, and non-String values raise TypeError.
• Documentation
  • Clarified that :binstr returns binary-safe strings. When passing input that may contain NUL bytes, provide its byte length separately.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: bf7276c5-85ca-4508-a6be-953312c36b90

---

📥 Commits

Reviewing files that changed from the base of the PR and between 91b8722 and 00b3f34.

---

📒 Files selected for processing (5)

• docs/FFI.md
• src/codegen_call.c
• src/codegen_call_class.c
• test/ffi_str_spec_aliases_arg.rb
• test/ffi_str_spec_aliases_arg.rb.expected

---

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

---

---

📝 Walkthrough

Walkthrough

FFI call conversion and callback boxing now use ffi_spec_is_str to recognize string-like specifications. Documentation and tests cover :string and :binstr arguments, including nil, non-String values, and callbacks.

Changes

FFI string specification aliases

Layer / File(s)	Summary
String specification handling and coverage src/codegen_call_class.c, src/codegen_call.c, docs/FFI.md, test/ffi_str_spec_aliases_arg.rb, test/ffi_str_spec_aliases_arg.rb.expected	FFI calls use ffi_spec_is_str to count and convert string arguments. Callback boxing uses the same helper. Documentation describes :binstr input and return behavior. The fixture tests :str, :string, and :binstr with strings, nil, non-String values, and callbacks.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

---

Merge Risk | ⚪ Minimal · up to 00b3f

Merge Risk: ⚪ Minimal · up to 00b3f

The FFI string-alias change has no identified merge-blocking issue. Complete the normal test checks before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 00b3f

The change aligns string aliases with existing native-call checks. No introduced security defect was established, but the lifetime of native buffers received through callbacks remains insufficiently demonstrated.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The visible affected boundary is between values in a compiled Ruby program and native functions or callback providers in that program. The supplied public-entrypoint ranges describe fixture declarations, not evidence of new tenant, network, or service exposure.

Trust Boundaries and Controls

• observed — The fixture covers boxed Integer rejection, nil arguments, mixed-container strings, and callback NULL handling. Its callbacks supply static literals and consume them immediately, so it does not demonstrate safe retention of transient provider-owned bytes. Boxed array insertion itself stores the value without duplicating string bytes.

Hardening Proposals

• proposed — Define the ownership contract for callback string parameters. If retaining callback values is supported, ensure provider bytes are copied before their lifetime ends and validate that transition using transient buffers. This addresses an existing ownership ambiguity, not an established PR-introduced vulnerability.

---

Pre-merge checks | 4 | 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (3 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly summarizes the main change: :binstr and :string arguments now accept String values like :str arguments.
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: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (3 skipped: 2 unsupported, 1 too large.)

---

• 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

---

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