Auto merge of #19925 - upsuper:cstr, r=emilio

Use cstr macro for ffi literal strings

Use `cstr!()` macro with `CStr` to ensure that literal strings used with FFI is properly nul-terminated to avoid cases like #19915.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19925)
<!-- Reviewable:end -->

Cargo.lock

@@ -565,6 +565,24 @@ dependencies = [
  "syn 0.11.11 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "cstr"
+version = "0.1.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "cstr-macros 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "procedural-masquerade 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
+[[package]]
+name = "cstr-macros"
+version = "0.1.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "procedural-masquerade 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "syn 0.12.12 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "darling"
 version = "0.3.0"
@@ -937,6 +955,7 @@ version = "0.0.1"
 dependencies = [
  "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "cssparser 0.23.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "cstr 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.33 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2982,6 +3001,7 @@ version = "0.0.1"
 dependencies = [
  "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "cssparser 0.23.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "cstr 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "env_logger 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.16.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "geckoservo 0.0.1",
@@ -3575,6 +3595,8 @@ dependencies = [
 "checksum core-text 9.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "2bd581c37283d0c23311d179aefbb891f2324ee0405da58a26e8594ab76e5748"
 "checksum cssparser 0.23.2 (registry+https://github.com/rust-lang/crates.io-index)" = "8a807ac3ab7a217829c2a3b65732b926b2befe6a35f33b4bf8b503692430f223"
 "checksum cssparser-macros 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "079adec4af52bb5275eadd004292028c79eb3c5f5b4ee8086a36d4197032f6df"
+"checksum cstr 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "b84061a2fd1af9319e98f733dc1ede67e0b208c455f17f1aa445cc8f4d0e635c"
+"checksum cstr-macros 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "f9f316203d1ea36f4f18316822806f6999aa3dc5ed1adf51e35b77e3b3933d78"
 "checksum darling 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d3effd06d4057f275cb7858889f4952920bab78dd8ff0f6e7dfe0c8d2e67ed89"
 "checksum darling_core 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "167dd3e235c2f1da16a635c282630452cdf49191eb05711de1bcd1d3d5068c00"
 "checksum darling_macro 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "c53edaba455f6073a10c27c72440860eb3f60444f8c8660a391032eeae744d82"

ports/geckolib/Cargo.toml

@@ -16,6 +16,7 @@ gecko_debug = ["style/gecko_debug"]
 [dependencies]
 atomic_refcell = "0.1"
 cssparser = "0.23.0"
+cstr = "0.1.2"
 env_logger = {version = "0.4", default-features = false} # disable `regex` to reduce code size
 libc = "0.2"
 log = {version = "0.3.5", features = ["release_max_level_info"]}

ports/geckolib/error_reporter.rs

@@ -9,6 +9,7 @@
 use cssparser::{CowRcStr, serialize_identifier, ToCss};
 use cssparser::{SourceLocation, ParseError, ParseErrorKind, Token, BasicParseErrorKind};
 use selectors::parser::SelectorParseErrorKind;
+use std::ffi::CStr;
 use std::ptr;
 use style::error_reporting::{ParseErrorReporter, ContextualParseError};
 use style::gecko_bindings::bindings::{Gecko_CreateCSSErrorReporter, Gecko_DestroyCSSErrorReporter};
@@ -76,7 +77,7 @@ enum Action {
 trait ErrorHelpers<'a> {
     fn error_data(self) -> (CowRcStr<'a>, ErrorKind<'a>);
     fn error_params(self) -> ErrorParams<'a>;
-    fn to_gecko_message(&self) -> (Option<&'static [u8]>, &'static [u8], Action);
+    fn to_gecko_message(&self) -> (Option<&'static CStr>, &'static CStr, Action);
 }
 
 fn extract_error_param<'a>(err: ErrorKind<'a>) -> Option<ErrorString<'a>> {
@@ -226,48 +227,48 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
         })
     }
 
-    fn to_gecko_message(&self) -> (Option<&'static [u8]>, &'static [u8], Action) {
+    fn to_gecko_message(&self) -> (Option<&'static CStr>, &'static CStr, Action) {
-        let (msg, action): (&[u8], Action) = match *self {
+        let (msg, action): (&CStr, Action) = match *self {
             ContextualParseError::UnsupportedPropertyDeclaration(
                 _, ParseError { kind: ParseErrorKind::Basic(BasicParseErrorKind::UnexpectedToken(_)), .. }
             ) |
             ContextualParseError::UnsupportedPropertyDeclaration(
                 _, ParseError { kind: ParseErrorKind::Basic(BasicParseErrorKind::AtRuleInvalid(_)), .. }
             ) => {
-                (b"PEParseDeclarationDeclExpected\0", Action::Skip)
+                (cstr!("PEParseDeclarationDeclExpected"), Action::Skip)
             }
             ContextualParseError::UnsupportedPropertyDeclaration(
                 _, ParseError { kind: ParseErrorKind::Custom(ref err), .. }
             ) => {
                 match *err {
                     StyleParseErrorKind::InvalidColor(_, _) => {
-                        return (Some(b"PEColorNotColor\0"),
+                        return (Some(cstr!("PEColorNotColor")),
-                                b"PEValueParsingError\0", Action::Drop)
+                                cstr!("PEValueParsingError"), Action::Drop)
                     }
                     StyleParseErrorKind::InvalidFilter(_, _) => {
-                        return (Some(b"PEExpectedNoneOrURLOrFilterFunction\0"),
+                        return (Some(cstr!("PEExpectedNoneOrURLOrFilterFunction")),
-                                b"PEValueParsingError\0", Action::Drop)
+                                cstr!("PEValueParsingError"), Action::Drop)
                     }
                     StyleParseErrorKind::OtherInvalidValue(_) => {
-                        (b"PEValueParsingError\0", Action::Drop)
+                        (cstr!("PEValueParsingError"), Action::Drop)
                     }
-                    _ => (b"PEUnknownProperty\0", Action::Drop)
+                    _ => (cstr!("PEUnknownProperty"), Action::Drop)
                 }
             }
             ContextualParseError::UnsupportedPropertyDeclaration(..) =>
-                (b"PEUnknownProperty\0", Action::Drop),
+                (cstr!("PEUnknownProperty"), Action::Drop),
             ContextualParseError::UnsupportedFontFaceDescriptor(..) =>
-                (b"PEUnknownFontDesc\0", Action::Skip),
+                (cstr!("PEUnknownFontDesc"), Action::Skip),
             ContextualParseError::InvalidKeyframeRule(..) =>
-                (b"PEKeyframeBadName\0", Action::Nothing),
+                (cstr!("PEKeyframeBadName"), Action::Nothing),
             ContextualParseError::UnsupportedKeyframePropertyDeclaration(..) =>
-                (b"PEBadSelectorKeyframeRuleIgnored\0", Action::Nothing),
+                (cstr!("PEBadSelectorKeyframeRuleIgnored"), Action::Nothing),
             ContextualParseError::InvalidRule(
                 _, ParseError { kind: ParseErrorKind::Custom(
                     StyleParseErrorKind::UnexpectedTokenWithinNamespace(_)
                 ), .. }
             ) => {
-                (b"PEAtNSUnexpected\0", Action::Nothing)
+                (cstr!("PEAtNSUnexpected"), Action::Nothing)
             }
             ContextualParseError::InvalidRule(
                 _, ParseError { kind: ParseErrorKind::Basic(BasicParseErrorKind::AtRuleInvalid(_)), .. }
@@ -277,84 +278,84 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
                     StyleParseErrorKind::UnsupportedAtRule(_)
                 ), .. }
             ) => {
-                (b"PEUnknownAtRule\0", Action::Nothing)
+                (cstr!("PEUnknownAtRule"), Action::Nothing)
             }
             ContextualParseError::InvalidRule(_, ref err) => {
                 let prefix = match err.kind {
                     ParseErrorKind::Custom(StyleParseErrorKind::SelectorError(ref err)) => match *err {
                         SelectorParseErrorKind::UnexpectedTokenInAttributeSelector(_) => {
-                            Some(&b"PEAttSelUnexpected\0"[..])
+                            Some(cstr!("PEAttSelUnexpected"))
                         }
                         SelectorParseErrorKind::ExpectedBarInAttr(_) => {
-                            Some(&b"PEAttSelNoBar\0"[..])
+                            Some(cstr!("PEAttSelNoBar"))
                         }
                         SelectorParseErrorKind::BadValueInAttr(_) => {
-                            Some(&b"PEAttSelBadValue\0"[..])
+                            Some(cstr!("PEAttSelBadValue"))
                         }
                         SelectorParseErrorKind::NoQualifiedNameInAttributeSelector(_) => {
-                            Some(&b"PEAttributeNameOrNamespaceExpected\0"[..])
+                            Some(cstr!("PEAttributeNameOrNamespaceExpected"))
                         }
                         SelectorParseErrorKind::InvalidQualNameInAttr(_) => {
-                            Some(&b"PEAttributeNameExpected\0"[..])
+                            Some(cstr!("PEAttributeNameExpected"))
                         }
                         SelectorParseErrorKind::ExplicitNamespaceUnexpectedToken(_) => {
-                            Some(&b"PETypeSelNotType\0"[..])
+                            Some(cstr!("PETypeSelNotType"))
                         }
                         SelectorParseErrorKind::ExpectedNamespace(_) => {
-                           Some(&b"PEUnknownNamespacePrefix\0"[..])
+                           Some(cstr!("PEUnknownNamespacePrefix"))
                         }
                         SelectorParseErrorKind::EmptySelector => {
-                            Some(&b"PESelectorGroupNoSelector\0"[..])
+                            Some(cstr!("PESelectorGroupNoSelector"))
                         }
                         SelectorParseErrorKind::DanglingCombinator => {
-                            Some(&b"PESelectorGroupExtraCombinator\0"[..])
+                            Some(cstr!("PESelectorGroupExtraCombinator"))
                         }
                         SelectorParseErrorKind::UnsupportedPseudoClassOrElement(_) => {
-                            Some(&b"PEPseudoSelUnknown\0"[..])
+                            Some(cstr!("PEPseudoSelUnknown"))
                         }
                         SelectorParseErrorKind::PseudoElementExpectedColon(_) => {
-                            Some(&b"PEPseudoSelEndOrUserActionPC\0"[..])
+                            Some(cstr!("PEPseudoSelEndOrUserActionPC"))
                         }
                         SelectorParseErrorKind::NoIdentForPseudo(_) => {
-                            Some(&b"PEPseudoClassArgNotIdent\0"[..])
+                            Some(cstr!("PEPseudoClassArgNotIdent"))
                         }
                         SelectorParseErrorKind::PseudoElementExpectedIdent(_) => {
-                            Some(&b"PEPseudoSelBadName\0"[..])
+                            Some(cstr!("PEPseudoSelBadName"))
                         }
                         SelectorParseErrorKind::ClassNeedsIdent(_) => {
-                            Some(&b"PEClassSelNotIdent\0"[..])
+                            Some(cstr!("PEClassSelNotIdent"))
                         }
                         SelectorParseErrorKind::EmptyNegation => {
-                            Some(&b"PENegationBadArg\0"[..])
+                            Some(cstr!("PENegationBadArg"))
                         }
                         _ => None,
                     },
                     _ => None,
                 };
-                return (prefix, b"PEBadSelectorRSIgnored\0", Action::Nothing);
+                return (prefix, cstr!("PEBadSelectorRSIgnored"), Action::Nothing);
             }
             ContextualParseError::InvalidMediaRule(_, ref err) => {
-                let err: &[u8] = match err.kind {
+                let err: &CStr = match err.kind {
                     ParseErrorKind::Custom(StyleParseErrorKind::ExpectedIdentifier(..)) => {
-                        b"PEGatherMediaNotIdent\0"
+                        cstr!("PEGatherMediaNotIdent")
                     },
                     ParseErrorKind::Custom(StyleParseErrorKind::MediaQueryExpectedFeatureName(..)) => {
-                        b"PEMQExpectedFeatureName\0"
+                        cstr!("PEMQExpectedFeatureName")
                     },
                     ParseErrorKind::Custom(StyleParseErrorKind::MediaQueryExpectedFeatureValue) => {
-                        b"PEMQExpectedFeatureValue\0"
+                        cstr!("PEMQExpectedFeatureValue")
                     },
                     ParseErrorKind::Custom(StyleParseErrorKind::RangedExpressionWithNoValue) => {
-                        b"PEMQNoMinMaxWithoutValue\0"
+                        cstr!("PEMQNoMinMaxWithoutValue")
                     },
                     _ => {
-                        b"PEDeclDropped\0"
+                        cstr!("PEDeclDropped")
                     },
                 };
                 (err, Action::Nothing)
             }
             ContextualParseError::UnsupportedRule(..) =>
-                (b"PEDeclDropped\0", Action::Nothing),
+                (cstr!("PEDeclDropped"), Action::Nothing),
             ContextualParseError::UnsupportedViewportDescriptorDeclaration(..) |
             ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(..) |
             ContextualParseError::InvalidCounterStyleWithoutSymbols(..) |
@@ -364,21 +365,21 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
             ContextualParseError::InvalidCounterStyleExtendsWithAdditiveSymbols |
             ContextualParseError::UnsupportedFontFeatureValuesDescriptor(..) |
             ContextualParseError::InvalidFontFeatureValuesRule(..) =>
-                (b"PEUnknownAtRule\0", Action::Skip),
+                (cstr!("PEUnknownAtRule"), Action::Skip),
             ContextualParseError::UnsupportedValue(_, ParseError { ref kind, .. }) => {
                 match *kind {
                     ParseErrorKind::Custom(
                         StyleParseErrorKind::ValueError(
                             ValueParseErrorKind::InvalidColor(..)
                         )
-                    ) => (b"PEColorNotColor\0", Action::Nothing),
+                    ) => (cstr!("PEColorNotColor"), Action::Nothing),
                     _ => {
                         // Not the best error message, since we weren't parsing
                         // a declaration, just a value. But we don't produce
                         // UnsupportedValue errors other than InvalidColors
                         // currently.
                         debug_assert!(false, "should use a more specific error message");
-                        (b"PEDeclDropped\0", Action::Nothing)
+                        (cstr!("PEDeclDropped"), Action::Nothing)
                     }
                 }
             }
@@ -392,8 +393,8 @@ impl ErrorReporter {
         let (pre, name, action) = error.to_gecko_message();
         let suffix = match action {
             Action::Nothing => ptr::null(),
-            Action::Skip => b"PEDeclSkipped\0".as_ptr(),
+            Action::Skip => cstr!("PEDeclSkipped").as_ptr(),
-            Action::Drop => b"PEDeclDropped\0".as_ptr(),
+            Action::Drop => cstr!("PEDeclDropped").as_ptr(),
         };
         let params = error.error_params();
         let param = params.main_param;

ports/geckolib/lib.rs

@@ -4,6 +4,7 @@
 
 
 extern crate cssparser;
+#[macro_use] extern crate cstr;
 extern crate env_logger;
 extern crate libc;
 #[macro_use] extern crate log;

ports/geckolib/tests/Cargo.toml

@@ -14,6 +14,7 @@ doctest = false
 [dependencies]
 atomic_refcell = "0.1"
 cssparser = "0.23.0"
+cstr = "0.1.2"
 env_logger = "0.4"
 euclid = "0.16"
 geckoservo = {path = "../../../ports/geckolib"}

ports/geckolib/tests/lib.rs

@@ -13,6 +13,7 @@
 
 extern crate atomic_refcell;
 extern crate cssparser;
+#[macro_use] extern crate cstr;
 extern crate env_logger;
 extern crate geckoservo;
 #[macro_use] extern crate log;