Bug 1546048 - Use std::ffi::CStr and the cstr crate instead of custom stuff in xpcom. r=lina
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 22 Apr 2019 05:09:23 +0000
changeset 470330 43c7c3f10a71a5bede2282db7dcd80d674cf237d
parent 470329 6cbc4908bc4bb8e260cb452d36d5a1c9abf4468f
child 470331 b783cd5203ea589bb7505852e5108ed142d2d37a
child 470334 a033f955b188dcf34133d865822803c27602cf46
push id112863
push usershindli@mozilla.com
push dateMon, 22 Apr 2019 09:53:25 +0000
treeherdermozilla-inbound@ab1da7fa2ad0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslina
bugs1546048
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1546048 - Use std::ffi::CStr and the cstr crate instead of custom stuff in xpcom. r=lina Also, this checks for strings with nulls, which is nice (though I guess an uncommon mistake). Differential Revision: https://phabricator.services.mozilla.com/D28312
Cargo.lock
toolkit/components/places/bookmark_sync/Cargo.toml
toolkit/components/places/bookmark_sync/src/lib.rs
toolkit/components/places/bookmark_sync/src/merger.rs
toolkit/components/xulstore/Cargo.toml
toolkit/components/xulstore/src/lib.rs
toolkit/components/xulstore/src/persist.rs
toolkit/components/xulstore/src/statics.rs
xpcom/rust/moz_task/src/lib.rs
xpcom/rust/xpcom/src/cstr.rs
xpcom/rust/xpcom/src/lib.rs
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1,10 +1,8 @@
-# This file is automatically @generated by Cargo.
-# It is not intended for manual editing.
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
 [[package]]
 name = "Inflector"
 version = "0.11.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -353,16 +351,17 @@ dependencies = [
  "byte-tools 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
 name = "bookmark_sync"
 version = "0.1.0"
 dependencies = [
  "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "cstr 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "dogear 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.51 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "moz_task 0.1.0",
  "nserror 0.1.0",
  "nsstring 0.1.0",
  "storage 0.1.0",
  "storage_variant 0.1.0",
@@ -3433,16 +3432,17 @@ dependencies = [
  "syn 0.13.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
 name = "xulstore"
 version = "0.1.0"
 dependencies = [
  "crossbeam-utils 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "cstr 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "failure 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.51 (registry+https://github.com/rust-lang/crates.io-index)",
  "lmdb-rkv 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "moz_task 0.1.0",
  "nserror 0.1.0",
  "nsstring 0.1.0",
--- a/toolkit/components/places/bookmark_sync/Cargo.toml
+++ b/toolkit/components/places/bookmark_sync/Cargo.toml
@@ -4,16 +4,17 @@ version = "0.1.0"
 authors = ["Lina Cambridge <lina@yakshaving.ninja>"]
 edition = "2018"
 
 [dependencies]
 atomic_refcell = "0.1"
 dogear = "0.2.3"
 libc = "0.2"
 log = "0.4"
+cstr = "0.1"
 moz_task = { path = "../../../../xpcom/rust/moz_task" }
 nserror = { path = "../../../../xpcom/rust/nserror" }
 nsstring = { path = "../../../../xpcom/rust/nsstring" }
 storage = { path = "../../../../storage/rust" }
 storage_variant = { path = "../../../../storage/variant" }
 xpcom = { path = "../../../../xpcom/rust/xpcom" }
 
 [dependencies.thin-vec]
--- a/toolkit/components/places/bookmark_sync/src/lib.rs
+++ b/toolkit/components/places/bookmark_sync/src/lib.rs
@@ -1,15 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #![allow(non_snake_case)]
 
 #[macro_use]
+extern crate cstr;
+#[macro_use]
 extern crate xpcom;
 
 mod driver;
 mod error;
 mod merger;
 mod store;
 
 use xpcom::{interfaces::mozISyncedBookmarksMerger, RefPtr};
--- a/toolkit/components/places/bookmark_sync/src/merger.rs
+++ b/toolkit/components/places/bookmark_sync/src/merger.rs
@@ -149,29 +149,29 @@ impl MergeTask {
                 mozISyncedBookmarksMirrorLogger::LEVEL_WARN => LevelFilter::Warn,
                 mozISyncedBookmarksMirrorLogger::LEVEL_DEBUG => LevelFilter::Debug,
                 mozISyncedBookmarksMirrorLogger::LEVEL_TRACE => LevelFilter::Trace,
                 _ => LevelFilter::Off,
             })
             .unwrap_or(LevelFilter::Off);
         let logger = match logger {
             Some(logger) => Some(ThreadPtrHolder::new(
-                c_str!("mozISyncedBookmarksMirrorLogger"),
+                cstr!("mozISyncedBookmarksMirrorLogger"),
                 logger,
             )?),
             None => None,
         };
         Ok(MergeTask {
             db: db.clone(),
             max_log_level,
             logger,
             local_time_millis: local_time_seconds * 1000,
             remote_time_millis: remote_time_seconds * 1000,
             weak_uploads,
-            callback: ThreadPtrHolder::new(c_str!("mozISyncedBookmarksMirrorCallback"), callback)?,
+            callback: ThreadPtrHolder::new(cstr!("mozISyncedBookmarksMirrorCallback"), callback)?,
             result: AtomicRefCell::default(),
         })
     }
 }
 
 impl Task for MergeTask {
     fn run(&self) {
         let mut db = self.db.clone();
--- a/toolkit/components/xulstore/Cargo.toml
+++ b/toolkit/components/xulstore/Cargo.toml
@@ -1,16 +1,17 @@
 [package]
 name = "xulstore"
 version = "0.1.0"
 authors = ["nobody@mozilla.org"]
 license = "MPL-2.0"
 
 [dependencies]
 crossbeam-utils = "0.6.3"
+cstr = "0.1"
 lazy_static = "1.0"
 libc = "0.2"
 lmdb-rkv = "0.11.2"
 log = "0.4"
 moz_task = { path = "../../../xpcom/rust/moz_task" }
 nsstring = { path = "../../../xpcom/rust/nsstring" }
 nserror = { path = "../../../xpcom/rust/nserror" }
 rkv = "0.9.3"
--- a/toolkit/components/xulstore/src/lib.rs
+++ b/toolkit/components/xulstore/src/lib.rs
@@ -1,14 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 extern crate crossbeam_utils;
 #[macro_use]
+extern crate cstr;
+#[macro_use]
 extern crate failure;
 #[macro_use]
 extern crate lazy_static;
 extern crate libc;
 extern crate lmdb;
 #[macro_use]
 extern crate log;
 extern crate moz_task;
--- a/toolkit/components/xulstore/src/persist.rs
+++ b/toolkit/components/xulstore/src/persist.rs
@@ -64,17 +64,17 @@ lazy_static! {
 }
 
 fn observe_xpcom_shutdown() {
     (|| -> XULStoreResult<()> {
         let obs_svc = xpcom::services::get_ObserverService().ok_or(XULStoreError::Unavailable)?;
         let observer = XpcomShutdownObserver::new();
         unsafe {
             obs_svc
-                .AddObserver(observer.coerce(), c_str!("xpcom-shutdown").as_ptr(), false)
+                .AddObserver(observer.coerce(), cstr!("xpcom-shutdown").as_ptr(), false)
                 .to_result()?
         };
         Ok(())
     })()
     .unwrap_or_else(|err| error!("error observing XPCOM shutdown: {}", err));
 }
 
 pub(crate) fn clear_on_shutdown() {
--- a/toolkit/components/xulstore/src/statics.rs
+++ b/toolkit/components/xulstore/src/statics.rs
@@ -74,25 +74,25 @@ fn get_profile_dir() -> XULStoreResult<P
     // a `*mut *mut libc::c_void` in Rust, whereas getter_addrefs() expects
     // a closure with a `*mut *const T` parameter.
 
     let dir_svc = xpcom::services::get_DirectoryService().ok_or(XULStoreError::Unavailable)?;
     let mut profile_dir = xpcom::GetterAddrefs::<nsIFile>::new();
     unsafe {
         dir_svc
             .Get(
-                c_str!("ProfD").as_ptr(),
+                cstr!("ProfD").as_ptr(),
                 &nsIFile::IID,
                 profile_dir.void_ptr(),
             )
             .to_result()
             .or_else(|_| {
                 dir_svc
                     .Get(
-                        c_str!("ProfDS").as_ptr(),
+                        cstr!("ProfDS").as_ptr(),
                         &nsIFile::IID,
                         profile_dir.void_ptr(),
                     )
                     .to_result()
             })?;
     }
     let profile_dir = profile_dir.refptr().ok_or(XULStoreError::Unavailable)?;
 
@@ -126,17 +126,17 @@ fn observe_profile_change() {
     (|| -> XULStoreResult<()> {
         // Observe profile changes so we can update this directory accordingly.
         let obs_svc = xpcom::services::get_ObserverService().ok_or(XULStoreError::Unavailable)?;
         let observer = ProfileChangeObserver::new();
         unsafe {
             obs_svc
                 .AddObserver(
                     observer.coerce(),
-                    c_str!("profile-after-change").as_ptr(),
+                    cstr!("profile-after-change").as_ptr(),
                     false,
                 )
                 .to_result()?
         };
         Ok(())
     })()
     .unwrap_or_else(|err| error!("error observing profile change: {}", err));
 }
--- a/xpcom/rust/moz_task/src/lib.rs
+++ b/xpcom/rust/moz_task/src/lib.rs
@@ -13,21 +13,22 @@ extern crate nsstring;
 extern crate xpcom;
 
 use nserror::{nsresult, NS_OK};
 use nsstring::{nsACString, nsCString};
 use std::{
     marker::PhantomData,
     mem, ptr,
     sync::atomic::{AtomicBool, Ordering},
+    ffi::CStr,
 };
 use xpcom::{
     getter_addrefs,
     interfaces::{nsIEventTarget, nsIRunnable, nsISupports, nsIThread},
-    xpcom, xpcom_method, AtomicRefcnt, NulTerminatedCStr, RefCounted, RefPtr, XpCom,
+    xpcom, xpcom_method, AtomicRefcnt, RefCounted, RefPtr, XpCom,
 };
 
 extern "C" {
     fn NS_GetCurrentThreadEventTarget(result: *mut *const nsIThread) -> nsresult;
     fn NS_GetMainThreadEventTarget(result: *mut *const nsIThread) -> nsresult;
     fn NS_IsMainThread() -> bool;
     fn NS_NewNamedThreadWithDefaultStackSize(
         name: *const nsACString,
@@ -136,17 +137,17 @@ impl TaskRunnable {
 pub type ThreadPtrHandle<T> = RefPtr<ThreadPtrHolder<T>>;
 
 /// A Rust analog to `nsMainThreadPtrHolder` that wraps an `nsISupports` object
 /// with thread-safe refcounting. The holder keeps one reference to the wrapped
 /// object that's released when the holder's refcount reaches zero.
 pub struct ThreadPtrHolder<T: XpCom + 'static> {
     ptr: *const T,
     marker: PhantomData<T>,
-    name: NulTerminatedCStr,
+    name: &'static CStr,
     owning_thread: RefPtr<nsIThread>,
     refcnt: AtomicRefcnt,
 }
 
 unsafe impl<T: XpCom + 'static> Send for ThreadPtrHolder<T> {}
 unsafe impl<T: XpCom + 'static> Sync for ThreadPtrHolder<T> {}
 
 unsafe impl<T: XpCom + 'static> RefCounted for ThreadPtrHolder<T> {
@@ -179,17 +180,17 @@ unsafe impl<T: XpCom + 'static> RefCount
             Box::from_raw(self as *const Self as *mut Self);
         }
     }
 }
 
 impl<T: XpCom + 'static> ThreadPtrHolder<T> {
     /// Creates a new owning thread pointer holder. Returns an error if the
     /// thread manager has shut down. Panics if `name` isn't a valid C string.
-    pub fn new(name: NulTerminatedCStr, ptr: RefPtr<T>) -> Result<RefPtr<Self>, nsresult> {
+    pub fn new(name: &'static CStr, ptr: RefPtr<T>) -> Result<RefPtr<Self>, nsresult> {
         let owning_thread = get_current_thread()?;
         // Take ownership of the `RefPtr`. This does _not_ decrement its
         // refcount, which is what we want. Once we've released all references
         // to the holder, we'll release the wrapped `RefPtr`.
         let raw: *const T = &*ptr;
         mem::forget(ptr);
         unsafe {
             let boxed = Box::new(ThreadPtrHolder {
deleted file mode 100644
--- a/xpcom/rust/xpcom/src/cstr.rs
+++ /dev/null
@@ -1,28 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-use libc::c_char;
-
-/// Creates a static C string by adding a nul terminator to `$str`.
-#[macro_export]
-macro_rules! c_str {
-    ($str:expr) => {
-        $crate::NulTerminatedCStr(concat!($str, '\0').as_bytes())
-    };
-}
-
-/// A nul-terminated, static C string. This should only be created via the
-/// `c_str` macro.
-pub struct NulTerminatedCStr(pub &'static [u8]);
-
-impl NulTerminatedCStr {
-    /// Returns a raw pointer to this string, asserting that it's
-    /// nul-terminated. This pointer can be passed to any C function expecting a
-    /// `const char*`, or any XPIDL method expecting an `in string`.
-    #[inline]
-    pub fn as_ptr(&self) -> *const c_char {
-        assert_eq!(self.0.last(), Some(&0), "C strings must be nul-terminated");
-        self.0 as *const [u8] as *const c_char
-    }
-}
--- a/xpcom/rust/xpcom/src/lib.rs
+++ b/xpcom/rust/xpcom/src/lib.rs
@@ -23,19 +23,16 @@ extern crate thin_vec;
 extern crate xpcom_macros;
 #[doc(hidden)]
 pub use xpcom_macros::*;
 
 // Helper functions and data structures are exported in the root of the crate.
 mod base;
 pub use base::*;
 
-mod cstr;
-pub use cstr::*;
-
 // Declarative macro to generate XPCOM method stubs.
 mod method;
 pub use method::*;
 
 mod refptr;
 pub use refptr::*;
 
 mod statics;