Lint str-ptr-in-c-abi discourage str pointers in C ABI fns by fpdotmonkey · Pull Request #17401 · rust-lang/rust-clippy

fpdotmonkey · 2026-07-10T18:43:07Z

Please write a short comment explaining your change (or "none" for internal only changes)

changelog: str-ptr-in-c-abi: This discourages using a pointer to a str instead of to a CString for extern "C" functions.

This closes #1236

rustbot · 2026-07-10T18:43:11Z

Thanks for the pull request, and welcome!You should hear from one of our reviewers after this PR is reviewed by at least 2 reviewers from the community

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

fpdotmonkey · 2026-07-10T20:32:19Z

I'm not really sure what the failure on Lintcheck is. The error messages sounds like failures in external crates, but that's almost certainly not the case.

Gri-ffin · 2026-07-10T21:09:54Z

You are currently using fn_sig unconditionally, you should check if it's an FnDef or FnPtr.
@rustbot author

rustbot · 2026-07-10T21:13:22Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

github-actions · 2026-07-10T21:45:34Z

Lintcheck changes for a996439

Lint	Added	Removed	Changed
`clippy::str_ptr_in_c_abi`	1	0	0

This comment will be updated if you push new changes

fpdotmonkey · 2026-07-10T21:47:45Z

You are currently using fn_sig unconditionally, you should check if it's an FnDef or FnPtr.

Fixed

@rustbot ready

rustbot · 2026-07-11T11:21:03Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

Gri-ffin

@rustbot author

View changes since this review

Gri-ffin · 2026-07-11T11:55:51Z

+        let ExprKind::Call(callee, args) = expr.kind else {
+            return;
+        };
+        // where the signature of the callee shows it is
+        // extern "C"
+        if is_extern_c_fn(cx, callee) {
+            // and takes a raw pointer as an argument
+            let arg_idxs = raw_pointer_arg_idxs(cx, callee);
+            if arg_idxs.is_empty() {
+                return;
+            }
+            let relevant_args = arg_idxs
+                .into_iter()
+                .filter_map(|idx| args.get(idx))
+                .map(Clone::clone)
+                .collect::<Vec<_>>();
+            let problem_args = args_str_ptr_cast(cx, &relevant_args);
+            if problem_args.is_empty() {
+                return;
+            }
+            let span: rustc_errors::MultiSpan = problem_args
+                .into_iter()
+                .map(|arg| arg.span)
+                .collect::<Vec<rustc_span::Span>>()
+                .into();


You can combine these checks into an if let chain and use an iterator that only collect at the end. (See review below). then you can pass the spans directly to span_lint_and_help (if they are not empty).

Gri-ffin · 2026-07-11T12:10:08Z

+/// Returns elements from `args` which have a Rust `str` as a raw pointer cast to a raw pointer.
+fn args_str_ptr_cast<'tcx>(cx: &LateContext<'tcx>, args: &[Expr<'tcx>]) -> Vec<Expr<'tcx>> {
+    args.iter()
+        .filter(|arg| {
+            // just a simple check for
+            // `let s: &str; libc::strlen(s.as_ptr() as *const _);`
+            // or similarly with `as_mut_ptr`/`*mut`
+            if let ExprKind::Cast(expr, ty) = arg.kind
+                && matches!(ty.kind, TyKind::Ptr(_))
+                && let ExprKind::MethodCall(method, this, _args, _span) = expr.kind
+                && ["as_ptr", "as_mut_ptr"].contains(&method.ident.name.as_str())
+                && is_ref_str(cx.typeck_results().expr_ty(this))
+            {
+                true
+            } else {
+                false
+            }
+        })
+        .map(Clone::clone)
+        .collect()
+}
+
+/// is `ty` either `&str` or `&mut str`?
+fn is_ref_str(ty: Ty<'_>) -> bool {
+    if let rustc_middle::ty::Ref(_, reffed_ty, _) = ty.kind() {
+        return reffed_ty.is_str();
+    }
+    false
+}


You can replace this with a single function that checks one argument and returns a bool if it's cast to a raw pointer (for example a is_str_ptr_cast that you can use in the above filter closure in the iterator), you can use expr_ty_adjusted to catch String, etc... instead of being limited to str.
Also you can use sym from clippy_utils instead of comparing strings directly, like this:

.... matches!(method.ident.name, sym::as_ptr | sym::as_mut_ptr)

Oh awesome, I didn't know expr_ty_adjusted would just give that for free.

Also you can use sym from clippy_utils instead of comparing strings directly, like this:

Alas, as_mut_ptr doesn't exist in sym, so I still have to use strings

You can add it there of course.

Oh I didn't consider that that was possible.

fpdotmonkey · 2026-07-12T14:11:24Z

@rustbot ready

Gri-ffin

And can you please change "This addresses" to "fixes" or "closes" in the PR description.
@rustbot author

View changes since this review

Gri-ffin · 2026-07-13T09:22:14Z

+/// The same signature as in the libc crate, but without variadics
+#[allow(unused)]
+unsafe extern "C" fn printf(format: *const i8) -> i32 {
+    unimplemented!()
+}


We should lint on variadic functions.

Suggested change

/// The same signature as in the libc crate, but without variadics

#[allow(unused)]

unsafe extern "C" fn printf(format: *const i8) -> i32 {

unimplemented!()

}

unsafe extern "C" {

fn printf(format: *const i8, ...);

}

Gri-ffin · 2026-07-13T09:24:17Z

+#[allow(unused)]
+unsafe extern "C" fn strcpy(dst: *mut i8, src: *const i8) -> *mut i8 {
+    unimplemented!()
+}
+


Suggested change

#[allow(unused)]

unsafe extern "C" fn strcpy(dst: *mut i8, src: *const i8) -> *mut i8 {

unimplemented!()

}

unsafe extern "C" {

fn strcpy(dst: *mut i8, src: *const i8) -> *mut i8;

}

}

Gri-ffin · 2026-07-13T09:46:18Z

+/// If argument `arg.0` of function `callee` is a raw ptr and `arg.1` is
+/// `derefs_to_str.as(_mut)_ptr() as *const(mut) _`, then true.
+fn is_cast_str_ptr_to_raw_ptr<'tcx>(
+    cx: &LateContext<'tcx>,
+    callee: &'tcx Expr<'tcx>,
+    arg: &(usize, &Expr<'tcx>),
+) -> bool {
+    let typeck = cx.typeck_results();
+    let callee_ty = typeck.expr_ty(callee);
+    if callee_ty.is_fn()
+        && callee_ty
+            .fn_sig(cx.tcx)
+            .inputs()
+            .iter()
+            .nth(arg.0)
+            .is_some_and(|ty| ty.skip_binder().is_raw_ptr())
+        && let ExprKind::Cast(expr, ty) = arg.1.kind
+        && matches!(ty.kind, TyKind::Ptr(_))
+        && let ExprKind::MethodCall(method, this, _args, _span) = expr.kind
+        // as_mut_ptr doesn't exist in clippy_utils::sym, so fall back to str
+        && [sym::as_ptr, sym::as_mut_ptr].contains(&method.ident.name)
+        && is_ref_str(typeck.expr_ty_adjusted(this))
+    {
+        return true;
+    }
+    false
+}


Instead of looking at the inputs() of the callee's signature, you can check the argument expression directly which would allow you to catch bad ptrs passed to variadic functions.
Also no need for is_ref_str function as .peel_refs().is_str() will automatically peel all & and &mut refs.
And its better to use matches! rather than ~~allocating an array~~ creating a temporary array and checking it using .contains (it's also the idiomatic way).
Basically something like this:

fn is_str_ptr_cast<'tcx>(cx: &LateContext<'tcx>, arg: &Expr<'tcx>) -> bool { // Check if arg is a cast // and if type is Ptr // and expression is a method call if so { does the method identifier name match as_ptr or as_mut_ptr and is the adjusted type after peeling refs an str? } else { false } }

Happy to change to using matches!, though I'm curious to probe at the reasoning. matches! and [&str; N]::contains appear to generate identical code at opt_level=2 for a toy example (https://godbolt.org/z/eKqj9Tb1j), certainly without allocations. Perhaps the problem is that Symbol adds complications, or that contains generates a lot more code at low optimization levels?

It's mostly that matches! is the idiom, and .contains would need a temporary array coerced to a slice and a method call (which will get optimized later). I wasn't clear enough.

Cool thanks for the clarification.

So if we're not checking against callee_ty.inputs(), does it make sense to extend this to slices since that should be exactly the same pattern? I suppose the error message should change based on whether it's a str or [T] and the name of the lint would need to change as well.

I would personally avoid that, I think it's best to keep it simple and keep the scope of this PR to str/C strings, feel free to raise this point on Zulip once the FCP starts.

Gri-ffin · 2026-07-13T09:59:05Z

+            && let str_ptr_args = args
+                .iter()
+                .enumerate()
+                .filter(|arg| is_cast_str_ptr_to_raw_ptr(cx, callee, arg))
+                .map(|(_, arg)| arg)
+                .collect::<Vec<_>>()
+            && !str_ptr_args.is_empty()
+        {
+            let span: rustc_errors::MultiSpan = str_ptr_args
+                .into_iter()
+                .map(|arg| arg.span)
+                .collect::<Vec<rustc_span::Span>>()
+                .into();


Since we no longer need the arg index, you can dropthe .enumerate, and no need for two allocations, you can combine these into a single iterator that just maps straight to the Span and collect the spans at the very end, and pass them directly to span_lint_and_help if they are not empty.

fpdotmonkey · 2026-07-13T11:43:17Z

And can you please change "This addresses" to "fixes" or "closes" in the PR description.

done

This discourages using a pointer to a `str` instead of to a CString for `extern "C"` functions.

Gri-ffin

Some nits and last comments and it looks good to me.

View changes since this review

Gri-ffin · 2026-07-14T08:55:31Z

+    /// ```
+    #[clippy::version = "1.99.0"]
+    pub STR_PTR_IN_C_ABI,
+    nursery,


You should choose a category for this lint, I would personally go with suspicious.

Gri-ffin · 2026-07-14T08:55:54Z

+            && is_extern_c_fn(cx, callee)
+        // and takes a raw pointer as an argument
+        // and to that argument gives a pointer to a Rust `str`
+            && let span = args


nit:

Suggested change

&& let span = args

&& let spans = args

Gri-ffin · 2026-07-14T09:04:43Z

+    /// Rust's `str` doesn't provide a null byte.  Instead it contains a length for the string.
+    /// This leads to two problems.
+    ///
+    /// 1. The length parameter of the Rust string will be misinterpreted as a character, which is logically invalid.


This is incorrect, the C function will not see or misinterpret rust length metadata as a character since str::as_ptr() only returns a pointer to the string’s UTF-8 bytes. Better to just remove this since the main problem is that C APIs can read past the string if no length is provided.

Gri-ffin · 2026-07-14T09:08:18Z

+                span,
+                "giving a pointer to a Rust `str` to an `extern \"C\" fn` can cause undefined behavior",
+                /* help_span */ None,
+                "first convert the `str` to a `std::ffi::CString` and then get a pointer from there",


I think this can be softened. Some FFI APIs take a pointer plus a length, so we can't always prove that converting to Cstring is the correct thing to do.

Something like ""if this function expects a nul-terminated string, convert the str to a CString first" would appear to me to be better.

rustbot added the S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. label Jul 10, 2026

rustbot added needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jul 10, 2026

fpdotmonkey force-pushed the lint-str-ptr-in-c-abi branch 3 times, most recently from db505ce to 0d5dddd Compare July 10, 2026 20:06

rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jul 10, 2026

fpdotmonkey force-pushed the lint-str-ptr-in-c-abi branch from 0d5dddd to 5f74d2d Compare July 10, 2026 21:36

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jul 10, 2026

This comment has been minimized.

Sign in to view

fpdotmonkey force-pushed the lint-str-ptr-in-c-abi branch from 5f74d2d to 9669223 Compare July 11, 2026 11:21

Gri-ffin reviewed Jul 11, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jul 11, 2026

fpdotmonkey force-pushed the lint-str-ptr-in-c-abi branch from 9669223 to 69849b8 Compare July 12, 2026 13:59

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Jul 12, 2026

fpdotmonkey force-pushed the lint-str-ptr-in-c-abi branch from 69849b8 to 0c0f639 Compare July 12, 2026 19:32

Gri-ffin reviewed Jul 13, 2026

View reviewed changes

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 13, 2026

rustbot added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Jul 13, 2026

fpdotmonkey force-pushed the lint-str-ptr-in-c-abi branch from 0c0f639 to e2d13b9 Compare July 14, 2026 06:52

Lint str-ptr-in-c-abi discourage str pointers in C ABI fns

a996439

This discourages using a pointer to a `str` instead of to a CString for `extern "C"` functions.

fpdotmonkey force-pushed the lint-str-ptr-in-c-abi branch from e2d13b9 to a996439 Compare July 14, 2026 06:54

Gri-ffin reviewed Jul 14, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

fpdotmonkey commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 10, 2026

Uh oh!

fpdotmonkey commented Jul 10, 2026

Uh oh!

Gri-ffin commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 10, 2026

Uh oh!

github-actions Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fpdotmonkey commented Jul 10, 2026

Uh oh!

This comment has been minimized.

rustbot commented Jul 11, 2026

Uh oh!

Gri-ffin left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gri-ffin Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fpdotmonkey commented Jul 12, 2026

Uh oh!

Gri-ffin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gri-ffin Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fpdotmonkey Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gri-ffin Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gri-ffin Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fpdotmonkey commented Jul 13, 2026

Uh oh!

Gri-ffin left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Gri-ffin Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

fpdotmonkey commented Jul 10, 2026 •

edited

Loading

Gri-ffin commented Jul 10, 2026 •

edited

Loading

github-actions Bot commented Jul 10, 2026 •

edited

Loading

Gri-ffin left a comment •

edited by rustbot

Loading

Gri-ffin Jul 11, 2026 •

edited

Loading

Gri-ffin left a comment •

edited

Loading

Gri-ffin Jul 13, 2026 •

edited

Loading

fpdotmonkey Jul 13, 2026 •

edited

Loading

Gri-ffin Jul 13, 2026 •

edited

Loading

Gri-ffin Jul 14, 2026 •

edited

Loading

Gri-ffin left a comment •

edited by rustbot

Loading

Gri-ffin Jul 14, 2026 •

edited

Loading