Generate CStrs instead of byte arrays. (#2471)

Author: reitermarkus

CHANGELOG.md

@@ -166,7 +166,13 @@
 
 ## Added
 
+* Added the `--generate-cstr` CLI flag to generate string constants as `&CStr`
+  instead of `&[u8]`. (Requires Rust 1.59 or higher.)
+
 ## Changed
+
+* Non-UTF-8 string constants are now generated as references (`&[u8; SIZE]`)
+  instead of arrays (`[u8; SIZE]`) to match UTF-8 strings.
 * Wrappers for static functions that return `void` no longer contain a `return`
   statement and only call the static function instead.
 
@@ -252,7 +258,7 @@
  * The return type is now ommited in signatures of functions returning `void`.
  * Updated the `clap` dependency for `bindgen-cli` to 4.
  * Rewrote the `bindgen-cli` argument parser which could introduce unexpected
-   behavior changes. 
+   behavior changes.
  * The `ParseCallbacks::add_derives` method now receives `DeriveInfo<'_>` as
    argument instead of a `&str`. This type also includes the kind of target type.
 

bindgen-cli/options.rs

@@ -156,6 +156,9 @@ struct BindgenCommand {
     /// Generate block signatures instead of void pointers.
     #[arg(long)]
     generate_block: bool,
+    /// Generate string constants as `&CStr` instead of `&[u8]`.
+    #[arg(long)]
+    generate_cstr: bool,
     /// Use extern crate instead of use for block.
     #[arg(long)]
     block_extern_crate: bool,
@@ -430,6 +433,7 @@ where
         no_recursive_allowlist,
         objc_extern_crate,
         generate_block,
+        generate_cstr,
         block_extern_crate,
         distrust_clang_mangling,
         builtins,
@@ -755,6 +759,10 @@ where
         builder = builder.generate_block(true);
     }
 
+    if generate_cstr {
+        builder = builder.generate_cstr(true);
+    }
+
     if block_extern_crate {
         builder = builder.block_extern_crate(true);
     }

bindgen-tests/tests/expectations/tests/libclang-5/constant-evaluate.rs

@@ -0,0 +1,5 @@
+// bindgen-flags: --rust-target=1.0
+
+const char* MY_STRING_UTF8 = "Hello, world!";
+const char* MY_STRING_INTERIOR_NULL = "Hello,\0World!";
+const char* MY_STRING_NON_UTF8 = "ABCDE\xFF";

bindgen-tests/tests/expectations/tests/libclang-9/constant-evaluate.rs

@@ -0,0 +1,5 @@
+// bindgen-flags: --rust-target=1.59 --generate-cstr
+
+const char* MY_STRING_UTF8 = "Hello, world!";
+const char* MY_STRING_INTERIOR_NULL = "Hello,\0World!";
+const char* MY_STRING_NON_UTF8 = "ABCDE\xFF";

bindgen-tests/tests/expectations/tests/macro_const.rs

@@ -249,12 +249,6 @@ pub(crate) mod ast_ty {
         quote!(#val)
     }
 
-    pub(crate) fn byte_array_expr(bytes: &[u8]) -> TokenStream {
-        let mut bytes: Vec<_> = bytes.to_vec();
-        bytes.push(0);
-        quote! { [ #(#bytes),* ] }
-    }
-
     pub(crate) fn cstr_expr(mut string: String) -> TokenStream {
         string.push('\0');
         let b = proc_macro2::Literal::byte_string(string.as_bytes());

bindgen-tests/tests/expectations/tests/macro_const_1_0.rs

@@ -57,9 +57,10 @@ use crate::{Entry, HashMap, HashSet};
 use std::borrow::Cow;
 use std::cell::Cell;
 use std::collections::VecDeque;
+use std::ffi::CStr;
 use std::fmt::{self, Write};
 use std::ops;
-use std::str::FromStr;
+use std::str::{self, FromStr};
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum CodegenError {
@@ -692,41 +693,48 @@ impl CodeGenerator for Var {
                     });
                 }
                 VarType::String(ref bytes) => {
-                    // Account the trailing zero.
-                    //
+                    let prefix = ctx.trait_prefix();
+
+                    let options = ctx.options();
+                    let rust_features = options.rust_features;
+
+                    let mut cstr_bytes = bytes.clone();
+                    cstr_bytes.push(0);
+                    let len = proc_macro2::Literal::usize_unsuffixed(
+                        cstr_bytes.len(),
+                    );
+                    let cstr = CStr::from_bytes_with_nul(&cstr_bytes).unwrap();
+
                     // TODO: Here we ignore the type we just made up, probably
                     // we should refactor how the variable type and ty ID work.
-                    let len = bytes.len() + 1;
-                    let ty = quote! {
-                        [u8; #len]
-                    };
+                    let array_ty = quote! { [u8; #len] };
+                    let cstr_ty = quote! { ::#prefix::ffi::CStr };
 
-                    match String::from_utf8(bytes.clone()) {
-                        Ok(string) => {
-                            let cstr = helpers::ast_ty::cstr_expr(string);
-                            if ctx
-                                .options()
-                                .rust_features
-                                .static_lifetime_elision
-                            {
-                                result.push(quote! {
-                                    #(#attrs)*
-                                    pub const #canonical_ident : &#ty = #cstr ;
-                                });
-                            } else {
-                                result.push(quote! {
-                                    #(#attrs)*
-                                    pub const #canonical_ident : &'static #ty = #cstr ;
-                                });
-                            }
-                        }
-                        Err(..) => {
-                            let bytes = helpers::ast_ty::byte_array_expr(bytes);
-                            result.push(quote! {
-                                #(#attrs)*
-                                pub const #canonical_ident : #ty = #bytes ;
-                            });
+                    let bytes = proc_macro2::Literal::byte_string(
+                        cstr.to_bytes_with_nul(),
+                    );
+
+                    if rust_features.const_cstr && options.generate_cstr {
+                        result.push(quote! {
+                            #(#attrs)*
+                            #[allow(unsafe_code)]
+                            pub const #canonical_ident: &#cstr_ty = unsafe {
+                                #cstr_ty::from_bytes_with_nul_unchecked(#bytes)
+                            };
+                        });
+                    } else {
+                        let lifetime = if rust_features.static_lifetime_elision
+                        {
+                            None
+                        } else {
+                            Some(quote! { 'static })
                         }
+                        .into_iter();
+
+                        result.push(quote! {
+                            #(#attrs)*
+                            pub const #canonical_ident: &#(#lifetime )*#array_ty = #bytes ;
+                        });
                     }
                 }
                 VarType::Float(f) => {

bindgen-tests/tests/expectations/tests/strings_array.rs

@@ -128,6 +128,9 @@ macro_rules! rust_target_base {
             /// Rust stable 1.47
             /// * `larger_arrays` ([Tracking issue](https://github.com/rust-lang/rust/pull/74060))
             => Stable_1_47 => 1.47;
+            /// Rust stable 1.59
+            /// * `CStr::from_bytes_with_nul_unchecked` in `const` contexts ([PR](https://github.com/rust-lang/rust/pull/54745))
+            => Stable_1_59 => 1.59;
             /// Rust stable 1.64
             ///  * `core_ffi_c` ([Tracking issue](https://github.com/rust-lang/rust/issues/94501))
             => Stable_1_64 => 1.64;
@@ -241,6 +244,9 @@ rust_feature_def!(
     Stable_1_47 {
         => larger_arrays;
     }
+    Stable_1_59 {
+        => const_cstr;
+    }
     Stable_1_64 {
         => core_ffi_c;
     }

bindgen-tests/tests/expectations/tests/strings_cstr.rs

@@ -1416,6 +1416,23 @@ options! {
         },
         as_args: "--generate-block",
     },
+    /// Whether to generate strings as `CStr`.
+    generate_cstr: bool {
+        methods: {
+            /// Set whether string constants should be generated as `&CStr` instead of `&[u8]`.
+            ///
+            /// A minimum Rust target of 1.59 is required for this to have any effect as support
+            /// for `CStr::from_bytes_with_nul_unchecked` in `const` contexts is needed.
+            ///
+            /// This option is disabled by default but will become enabled by default in a future
+            /// release, so enabling this is recommended.
+            pub fn generate_cstr(mut self, doit: bool) -> Self {
+                self.options.generate_cstr = doit;
+                self
+            }
+        },
+        as_args: "--generate-cstr",
+    },
     /// Whether to emit `#[macro_use] extern crate block;` instead of `use block;` in the prologue
     /// of the files generated from apple block files.
     block_extern_crate: bool {