From 43c1aff738bf26cfe20eac0ab7c38b240defa2e5 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 22 Nov 2021 16:51:21 +0800 Subject: [PATCH] refactor(napi): use CStr instread of CString while creating lit variable --- crates/backend/src/codegen/const.rs | 21 +++++---- crates/backend/src/codegen/enum.rs | 46 +++++++++--------- crates/backend/src/codegen/fn.rs | 47 ++++++++++--------- crates/backend/src/codegen/struct.rs | 4 +- crates/napi/src/async_work.rs | 5 +- .../src/bindgen_runtime/js_values/promise.rs | 7 ++- .../src/bindgen_runtime/module_register.rs | 11 +++-- crates/napi/src/error.rs | 10 ++-- crates/napi/src/lib.rs | 1 - crates/napi/src/promise.rs | 4 +- 10 files changed, 85 insertions(+), 71 deletions(-) diff --git a/crates/backend/src/codegen/const.rs b/crates/backend/src/codegen/const.rs index ae4d04d4..cddad455 100644 --- a/crates/backend/src/codegen/const.rs +++ b/crates/backend/src/codegen/const.rs @@ -1,4 +1,4 @@ -use proc_macro2::{Literal, TokenStream}; +use proc_macro2::{Ident, Literal, TokenStream}; use quote::ToTokens; use crate::{codegen::get_register_ident, BindgenResult, NapiConst, TryToTokens}; @@ -19,22 +19,23 @@ impl NapiConst { fn gen_module_register(&self) -> TokenStream { let name_str = self.name.to_string(); let name_ident = self.name.clone(); - let js_name_lit = Literal::string(&self.js_name); + let js_name_lit = Literal::string(&format!("{}\0", self.name)); let register_name = get_register_ident(&name_str); let type_name = &self.type_name; + let cb_name = Ident::new( + &format!("__register__const__{}_callback__", register_name), + self.name.span(), + ); + quote! { + unsafe fn #cb_name(env: napi::sys::napi_env) -> napi::Result { + <#type_name as napi::bindgen_prelude::ToNapiValue>::to_napi_value(env, #name_ident) + } #[allow(non_snake_case)] #[allow(clippy::all)] #[napi::bindgen_prelude::ctor] fn #register_name() { - use std::ffi::CString; - use std::ptr; - - unsafe fn cb(env: napi::sys::napi_env) -> napi::Result { - <#type_name as napi::bindgen_prelude::ToNapiValue>::to_napi_value(env, #name_ident) - } - - napi::bindgen_prelude::register_module_export(#js_name_lit, cb); + napi::bindgen_prelude::register_module_export(#js_name_lit, #cb_name); } } } diff --git a/crates/backend/src/codegen/enum.rs b/crates/backend/src/codegen/enum.rs index 0ab5b5f9..bb1664aa 100644 --- a/crates/backend/src/codegen/enum.rs +++ b/crates/backend/src/codegen/enum.rs @@ -1,4 +1,4 @@ -use proc_macro2::{Literal, TokenStream}; +use proc_macro2::{Ident, Literal, Span, TokenStream}; use quote::ToTokens; use crate::{codegen::get_register_ident, BindgenResult, NapiEnum, TryToTokens}; @@ -98,18 +98,18 @@ impl NapiEnum { fn gen_module_register(&self) -> TokenStream { let name_str = self.name.to_string(); - let js_name_lit = Literal::string(&self.js_name); + let js_name_lit = Literal::string(&format!("{}\0", &self.js_name)); let register_name = get_register_ident(&name_str); let mut define_properties = vec![]; for variant in self.variants.iter() { - let name_lit = Literal::string(&variant.name.to_string()); + let name_lit = Literal::string(&format!("{}\0", variant.name.to_string())); let val_lit = Literal::i32_unsuffixed(variant.val); define_properties.push(quote! { { - let name = CString::new(#name_lit)?; + let name = std::ffi::CStr::from_bytes_with_nul_unchecked(#name_lit.as_bytes()); napi::bindgen_prelude::check_status!( napi::bindgen_prelude::sys::napi_set_named_property(env, obj_ptr, name.as_ptr(), i32::to_napi_value(env, #val_lit)?), "Failed to defined enum `{}`", @@ -119,28 +119,32 @@ impl NapiEnum { }) } + let callback_name = Ident::new( + &format!("__register__enum__{}_callback__", name_str), + Span::call_site(), + ); + quote! { + unsafe fn #callback_name(env: napi::bindgen_prelude::sys::napi_env) -> napi::bindgen_prelude::Result { + use std::ffi::CString; + use std::ptr; + + let mut obj_ptr = ptr::null_mut(); + + napi::bindgen_prelude::check_status!( + napi::bindgen_prelude::sys::napi_create_object(env, &mut obj_ptr), + "Failed to create napi object" + )?; + + #(#define_properties)* + + Ok(obj_ptr) + } #[allow(non_snake_case)] #[allow(clippy::all)] #[napi::bindgen_prelude::ctor] fn #register_name() { - use std::ffi::CString; - use std::ptr; - - unsafe fn cb(env: napi::bindgen_prelude::sys::napi_env) -> napi::bindgen_prelude::Result { - let mut obj_ptr = ptr::null_mut(); - - napi::bindgen_prelude::check_status!( - napi::bindgen_prelude::sys::napi_create_object(env, &mut obj_ptr), - "Failed to create napi object" - )?; - - #(#define_properties)* - - Ok(obj_ptr) - } - - napi::bindgen_prelude::register_module_export(#js_name_lit, cb); + napi::bindgen_prelude::register_module_export(#js_name_lit, #callback_name); } } } diff --git a/crates/backend/src/codegen/fn.rs b/crates/backend/src/codegen/fn.rs index 7ed24761..3261d32b 100644 --- a/crates/backend/src/codegen/fn.rs +++ b/crates/backend/src/codegen/fn.rs @@ -290,36 +290,41 @@ impl NapiFn { quote! {} } else { let name_str = self.name.to_string(); - let js_name = &self.js_name; + let js_name = format!("{}\0", &self.js_name); let name_len = js_name.len(); let module_register_name = get_register_ident(&name_str); let intermediate_ident = get_intermediate_ident(&name_str); + let cb_name = Ident::new( + &format!("__register__fn__{}_callback__", name_str), + Span::call_site(), + ); + quote! { + unsafe fn #cb_name(env: napi::bindgen_prelude::sys::napi_env) -> napi::bindgen_prelude::Result { + let mut fn_ptr = std::ptr::null_mut(); + + napi::bindgen_prelude::check_status!( + napi::bindgen_prelude::sys::napi_create_function( + env, + #js_name.as_ptr() as *const _, + #name_len, + Some(#intermediate_ident), + std::ptr::null_mut(), + &mut fn_ptr, + ), + "Failed to register function `{}`", + #name_str, + )?; + + Ok(fn_ptr) + } + #[allow(clippy::all)] #[allow(non_snake_case)] #[napi::bindgen_prelude::ctor] fn #module_register_name() { - unsafe fn cb(env: napi::bindgen_prelude::sys::napi_env) -> napi::bindgen_prelude::Result { - let mut fn_ptr = std::ptr::null_mut(); - - napi::bindgen_prelude::check_status!( - napi::bindgen_prelude::sys::napi_create_function( - env, - #js_name.as_ptr() as *const _, - #name_len, - Some(#intermediate_ident), - std::ptr::null_mut(), - &mut fn_ptr, - ), - "Failed to register function `{}`", - #name_str, - )?; - - Ok(fn_ptr) - } - - napi::bindgen_prelude::register_module_export(#js_name, cb); + napi::bindgen_prelude::register_module_export(#js_name, #cb_name); } } } diff --git a/crates/backend/src/codegen/struct.rs b/crates/backend/src/codegen/struct.rs index 4e8ebef8..b24cb6f7 100644 --- a/crates/backend/src/codegen/struct.rs +++ b/crates/backend/src/codegen/struct.rs @@ -378,7 +378,7 @@ impl NapiStruct { fn gen_register(&self) -> TokenStream { let name_str = self.name.to_string(); let struct_register_name = get_register_ident(&format!("{}_struct", name_str)); - let js_name = &self.js_name; + let js_name = format!("{}\0", self.js_name); let mut props = vec![]; if self.kind == NapiStructKind::Constructor { @@ -436,7 +436,7 @@ impl TryToTokens for NapiImpl { impl NapiImpl { fn gen_helper_mod(&self) -> BindgenResult { let name_str = self.name.to_string(); - let js_name = &self.js_name; + let js_name = format!("{}\0", self.js_name); let mod_name = Ident::new( &format!("__napi_impl_helper__{}", name_str), Span::call_site(), diff --git a/crates/napi/src/async_work.rs b/crates/napi/src/async_work.rs index 1ead56b4..dd44b4f5 100644 --- a/crates/napi/src/async_work.rs +++ b/crates/napi/src/async_work.rs @@ -1,8 +1,9 @@ +use std::ffi::CStr; use std::mem; use std::os::raw::c_void; use std::ptr; +use std::rc::Rc; use std::sync::atomic::{AtomicU8, Ordering}; -use std::{ffi::CString, rc::Rc}; use crate::{ bindgen_runtime::ToNapiValue, check_status, js_values::NapiValue, sys, Env, JsError, JsObject, @@ -59,7 +60,7 @@ pub fn run( napi_async_work: ptr::null_mut(), status: task_status.clone(), })); - let async_work_name = CString::new("napi_rs_async_work")?; + let async_work_name = unsafe { CStr::from_bytes_with_nul_unchecked(b"napi_rs_async_work\0") }; check_status!(unsafe { sys::napi_create_async_work( env, diff --git a/crates/napi/src/bindgen_runtime/js_values/promise.rs b/crates/napi/src/bindgen_runtime/js_values/promise.rs index c1604b35..bcd300ef 100644 --- a/crates/napi/src/bindgen_runtime/js_values/promise.rs +++ b/crates/napi/src/bindgen_runtime/js_values/promise.rs @@ -1,4 +1,4 @@ -use std::ffi::{c_void, CString}; +use std::ffi::{c_void, CStr}; use std::future; use std::pin::Pin; use std::ptr; @@ -15,7 +15,6 @@ pub struct Promise { } unsafe impl Send for Promise {} -unsafe impl Sync for Promise {} impl FromNapiValue for Promise { unsafe fn from_napi_value( @@ -23,7 +22,7 @@ impl FromNapiValue for Promise { napi_val: napi_sys::napi_value, ) -> crate::Result { let mut then = ptr::null_mut(); - let then_c_string = CString::new("then")?; + let then_c_string = CStr::from_bytes_with_nul_unchecked(b"then\0"); check_status!( napi_sys::napi_get_named_property(env, napi_val, then_c_string.as_ptr(), &mut then,), "Failed to get then function" @@ -55,7 +54,7 @@ impl FromNapiValue for Promise { "Failed to call then method" )?; let mut catch = ptr::null_mut(); - let catch_c_string = CString::new("catch")?; + let catch_c_string = CStr::from_bytes_with_nul_unchecked(b"catch\0"); check_status!( napi_sys::napi_get_named_property( env, diff --git a/crates/napi/src/bindgen_runtime/module_register.rs b/crates/napi/src/bindgen_runtime/module_register.rs index 77597a7a..89413ce7 100644 --- a/crates/napi/src/bindgen_runtime/module_register.rs +++ b/crates/napi/src/bindgen_runtime/module_register.rs @@ -1,4 +1,7 @@ -use std::{cell::RefCell, collections::HashMap, ffi::CString, ptr}; +use std::cell::RefCell; +use std::collections::HashMap; +use std::ffi::CStr; +use std::ptr; use crate::{check_status, check_status_or_throw, sys, JsError, Property, Result}; @@ -56,9 +59,9 @@ unsafe extern "C" fn napi_register_module_v1( MODULE_REGISTER_CALLBACK.with(|to_register_exports| { to_register_exports .take() - .into_iter() + .iter() .for_each(|(name, callback)| { - let js_name = CString::new(name).unwrap(); + let js_name = CStr::from_bytes_with_nul_unchecked(name.as_bytes()); unsafe { if let Err(e) = callback(env).and_then(|v| { check_status!( @@ -85,7 +88,7 @@ unsafe extern "C" fn napi_register_module_v1( let ctor = ctor.get(0).map(|c| c.raw().method.unwrap()).unwrap_or(noop); let raw_props: Vec<_> = props.iter().map(|prop| prop.raw()).collect(); - let js_class_name = CString::new(js_name).unwrap(); + let js_class_name = CStr::from_bytes_with_nul_unchecked(js_name.as_bytes()); let mut class_ptr = ptr::null_mut(); check_status_or_throw!( diff --git a/crates/napi/src/error.rs b/crates/napi/src/error.rs index 714b2465..4d9e4e92 100644 --- a/crates/napi/src/error.rs +++ b/crates/napi/src/error.rs @@ -1,6 +1,6 @@ use std::convert::{From, TryFrom}; use std::error; -use std::ffi::CString; +use std::ffi::{CStr, CString}; use std::fmt; #[cfg(feature = "serde-json")] use std::fmt::Display; @@ -206,11 +206,13 @@ macro_rules! impl_object_methods { } pub fn throw(&self, env: sys::napi_env) -> Result<()> { - let error_status = format!("{:?}", self.0.status); + let error_status = format!("{:?}\0", self.0.status); let status_len = error_status.len(); - let error_code_string = CString::new(error_status).unwrap(); + let error_code_string = + unsafe { CStr::from_bytes_with_nul_unchecked(error_status.as_bytes()) }; let reason_len = self.0.reason.len(); - let reason = CString::new(self.0.reason.clone()).unwrap(); + let reason_c_string = format!("{}\0", self.0.reason.clone()); + let reason = unsafe { CStr::from_bytes_with_nul_unchecked(reason_c_string.as_bytes()) }; let mut error_code = ptr::null_mut(); let mut reason_string = ptr::null_mut(); let mut js_error = ptr::null_mut(); diff --git a/crates/napi/src/lib.rs b/crates/napi/src/lib.rs index fcc7d3d5..82ec5609 100644 --- a/crates/napi/src/lib.rs +++ b/crates/napi/src/lib.rs @@ -156,7 +156,6 @@ macro_rules! assert_type_of { } #[allow(dead_code)] -#[cfg(debug_assertions)] pub(crate) unsafe fn log_js_value>( // `info`, `log`, `warning` or `error` method: &str, diff --git a/crates/napi/src/promise.rs b/crates/napi/src/promise.rs index 661ed1ce..e8d33395 100644 --- a/crates/napi/src/promise.rs +++ b/crates/napi/src/promise.rs @@ -1,4 +1,4 @@ -use std::ffi::CString; +use std::ffi::CStr; use std::future::Future; use std::marker::PhantomData; use std::os::raw::c_void; @@ -25,7 +25,7 @@ impl Result> { pub fn new(env: sys::napi_env, deferred: sys::napi_deferred, resolver: Resolver) -> Result { let mut async_resource_name = ptr::null_mut(); - let s = CString::new("napi_resolve_promise_from_future")?; + let s = unsafe { CStr::from_bytes_with_nul_unchecked(b"napi_resolve_promise_from_future\0") }; check_status!(unsafe { sys::napi_create_string_utf8(env, s.as_ptr(), 32, &mut async_resource_name) })?;