glib: Use TryFrom for GString conversions

gio/src/file.rs

@@ -237,7 +237,7 @@ impl<O: IsA<File>> FileExtManual for O {
                 + 'static,
         >,
     > {
-        let etag = etag.map(glib::GString::from);
+        let etag = etag.map(|e| glib::GString::try_from(e).unwrap());
         Box::pin(crate::GioFuture::new(
             self,
             move |obj, cancellable, send| {

glib/src/gstring.rs

@@ -33,7 +33,7 @@ impl GStr {
     pub fn from_str_with_nul(s: &str) -> Result<&Self, std::ffi::FromBytesWithNulError> {
         let bytes = s.as_bytes();
         CStr::from_bytes_with_nul(bytes)?;
-        Ok(unsafe { Self::from_bytes_with_nul_unchecked(bytes) })
+        Ok(unsafe { Self::from_utf8_with_nul_unchecked(bytes) })
     }
     // rustdoc-stripper-ignore-next
     /// Unsafely creates a GLib string wrapper from a byte slice.
@@ -42,7 +42,7 @@ impl GStr {
     /// sanity checks. The provided slice **must** be valid UTF-8, nul-terminated and not contain
     /// any interior nul bytes.
     #[inline]
-    pub const unsafe fn from_bytes_with_nul_unchecked(bytes: &[u8]) -> &Self {
+    pub const unsafe fn from_utf8_with_nul_unchecked(bytes: &[u8]) -> &Self {
         debug_assert!(!bytes.is_empty() && bytes[bytes.len() - 1] == 0);
         mem::transmute(bytes)
     }
@@ -53,7 +53,7 @@ impl GStr {
     #[inline]
     pub unsafe fn from_ptr<'a>(ptr: *const c_char) -> &'a Self {
         let cstr = CStr::from_ptr(ptr);
-        Self::from_bytes_with_nul_unchecked(cstr.to_bytes_with_nul())
+        Self::from_utf8_with_nul_unchecked(cstr.to_bytes_with_nul())
     }
     // rustdoc-stripper-ignore-next
     /// Converts this GLib string to a byte slice containing the trailing 0 byte.
@@ -126,7 +126,7 @@ impl GStr {
 #[macro_export]
 macro_rules! gstr {
     ($s:literal) => {
-        unsafe { $crate::GStr::from_bytes_with_nul_unchecked($crate::cstr_bytes!($s)) }
+        unsafe { $crate::GStr::from_utf8_with_nul_unchecked($crate::cstr_bytes!($s)) }
     };
 }
 
@@ -142,7 +142,7 @@ impl<'a> TryFrom<&'a CStr> for &'a GStr {
     #[inline]
     fn try_from(s: &'a CStr) -> Result<Self, Self::Error> {
         s.to_str()?;
-        Ok(unsafe { GStr::from_bytes_with_nul_unchecked(s.to_bytes_with_nul()) })
+        Ok(unsafe { GStr::from_utf8_with_nul_unchecked(s.to_bytes_with_nul()) })
     }
 }
 
@@ -270,7 +270,7 @@ unsafe impl<'a> crate::value::FromValue<'a> for &'a GStr {
         let ptr = gobject_ffi::g_value_get_string(value.to_glib_none().0);
         let cstr = CStr::from_ptr(ptr);
         assert!(cstr.to_str().is_ok());
-        GStr::from_bytes_with_nul_unchecked(cstr.to_bytes_with_nul())
+        GStr::from_utf8_with_nul_unchecked(cstr.to_bytes_with_nul())
     }
 }
 
@@ -388,7 +388,7 @@ impl GString {
                 slice::from_raw_parts(ptr.as_ptr() as *const _, len + 1)
             },
         };
-        unsafe { GStr::from_bytes_with_nul_unchecked(bytes) }
+        unsafe { GStr::from_utf8_with_nul_unchecked(bytes) }
     }
 
     // rustdoc-stripper-ignore-next
@@ -426,7 +426,7 @@ impl GString {
 
 impl Clone for GString {
     fn clone(&self) -> GString {
-        self.as_str().into()
+        self.as_gstr().to_owned()
     }
 }
 
@@ -651,23 +651,23 @@ impl From<GString> for Box<str> {
     }
 }
 
-impl From<String> for GString {
+impl TryFrom<String> for GString {
+    type Error = std::ffi::NulError;
     #[inline]
-    fn from(s: String) -> Self {
+    fn try_from(s: String) -> Result<Self, Self::Error> {
         // Moves the content of the String
-        unsafe {
-            // No check for valid UTF-8 here
-            let cstr = CString::from_vec_unchecked(s.into_bytes());
-            GString(Inner::Native(Some(cstr)))
-        }
+        // Check for interior nul bytes
+        let cstr = CString::new(s.into_bytes())?;
+        Ok(GString(Inner::Native(Some(cstr))))
     }
 }
 
-impl From<Box<str>> for GString {
+impl TryFrom<Box<str>> for GString {
+    type Error = std::ffi::NulError;
     #[inline]
-    fn from(s: Box<str>) -> Self {
+    fn try_from(s: Box<str>) -> Result<Self, Self::Error> {
         // Moves the content of the String
-        s.into_string().into()
+        s.into_string().try_into()
     }
 }
 
@@ -678,50 +678,69 @@ impl From<&GStr> for GString {
     }
 }
 
-impl From<&str> for GString {
+impl TryFrom<&str> for GString {
+    type Error = std::ffi::FromBytesWithNulError;
     #[inline]
-    fn from(s: &str) -> Self {
+    fn try_from(s: &str) -> Result<Self, Self::Error> {
         // Allocates with the GLib allocator
         unsafe {
             // No check for valid UTF-8 here
             let copy = ffi::g_malloc(s.len() + 1) as *mut c_char;
             ptr::copy_nonoverlapping(s.as_ptr() as *const c_char, copy, s.len() + 1);
             ptr::write(copy.add(s.len()), 0);
 
-            GString(Inner::Foreign {
+            // Check for interior nul bytes
+            let check = std::ffi::CStr::from_bytes_with_nul(std::slice::from_raw_parts(
+                copy as *const _,
+                s.len() + 1,
+            ));
+            if let Err(err) = check {
+                ffi::g_free(copy as *mut _);
+                return Err(err);
+            }
+
+            Ok(GString(Inner::Foreign {
                 ptr: ptr::NonNull::new_unchecked(copy),
                 len: s.len(),
-            })
+            }))
         }
     }
 }
 
-impl From<Vec<u8>> for GString {
+#[derive(thiserror::Error, Debug)]
+pub enum GStringError {
+    #[error("invalid UTF-8")]
+    Utf8(#[from] std::str::Utf8Error),
+    #[error("interior nul bytes")]
+    Nul(#[from] std::ffi::NulError),
+}
+
+impl TryFrom<Vec<u8>> for GString {
+    type Error = GStringError;
     #[inline]
-    fn from(s: Vec<u8>) -> Self {
+    fn try_from(s: Vec<u8>) -> Result<Self, Self::Error> {
         // Moves the content of the Vec
-        // Also check if it's valid UTF-8
-        let cstring = CString::new(s).expect("CString::new failed");
-        cstring.into()
+        // Also check if it's valid UTF-8 and has no interior nuls
+        let cstring = CString::new(s)?;
+        Ok(cstring.try_into()?)
     }
 }
 
-impl From<CString> for GString {
-    #[inline]
-    fn from(s: CString) -> Self {
+impl TryFrom<CString> for GString {
+    type Error = std::str::Utf8Error;
+    fn try_from(s: CString) -> Result<Self, Self::Error> {
         // Moves the content of the CString
         // Also check if it's valid UTF-8
-        assert!(s.to_str().is_ok());
-        Self(Inner::Native(Some(s)))
+        s.to_str()?;
+        Ok(Self(Inner::Native(Some(s))))
     }
 }
 
-impl From<&CStr> for GString {
+impl TryFrom<&CStr> for GString {
+    type Error = std::str::Utf8Error;
     #[inline]
-    fn from(c: &CStr) -> Self {
-        // Creates a copy with the GLib allocator
-        // Also check if it's valid UTF-8
-        c.to_str().unwrap().into()
+    fn try_from(c: &CStr) -> Result<Self, Self::Error> {
+        c.to_owned().try_into()
     }
 }
 
@@ -772,7 +791,7 @@ impl FromGlibPtrNone<*const u8> for GString {
         assert!(!ptr.is_null());
         let cstr = CStr::from_ptr(ptr as *const _);
         // Also check if it's valid UTF-8
-        cstr.to_str().unwrap().into()
+        cstr.try_into().unwrap()
     }
 }
 
@@ -904,16 +923,16 @@ impl<'a> ToGlibPtr<'a, *mut i8> for GString {
 impl<'a> FromGlibContainer<*const c_char, *const i8> for GString {
     unsafe fn from_glib_none_num(ptr: *const i8, num: usize) -> Self {
         if num == 0 || ptr.is_null() {
-            return Self::from("");
+            return Self::try_from("").unwrap();
         }
         let slice = slice::from_raw_parts(ptr as *const u8, num);
         // Also check if it's valid UTF-8
-        std::str::from_utf8(slice).unwrap().into()
+        std::str::from_utf8(slice).unwrap().try_into().unwrap()
     }
 
     unsafe fn from_glib_container_num(ptr: *const i8, num: usize) -> Self {
         if num == 0 || ptr.is_null() {
-            return Self::from("");
+            return Self::try_from("").unwrap();
         }
 
         // Check if it's valid UTF-8
@@ -928,7 +947,7 @@ impl<'a> FromGlibContainer<*const c_char, *const i8> for GString {
 
     unsafe fn from_glib_full_num(ptr: *const i8, num: usize) -> Self {
         if num == 0 || ptr.is_null() {
-            return Self::from("");
+            return Self::try_from("").unwrap();
         }
 
         // Check if it's valid UTF-8
@@ -1007,7 +1026,7 @@ unsafe impl<'a> crate::value::FromValue<'a> for GString {
     type Checker = crate::value::GenericValueTypeOrNoneChecker<Self>;
 
     unsafe fn from_value(value: &'a crate::Value) -> Self {
-        Self::from(<&str>::from_value(value))
+        Self::try_from(<&str>::from_value(value)).unwrap()
     }
 }
 
@@ -1097,15 +1116,15 @@ mod tests {
 
     #[test]
     fn test_gstring_from_str() {
-        let gstring: GString = "foo".into();
+        let gstring: GString = "foo".try_into().unwrap();
         assert_eq!(gstring.as_str(), "foo");
         let foo: Box<str> = gstring.into();
         assert_eq!(foo.as_ref(), "foo");
     }
 
     #[test]
     fn test_string_from_gstring() {
-        let gstring = GString::from("foo");
+        let gstring = GString::try_from("foo").unwrap();
         assert_eq!(gstring.as_str(), "foo");
         let s = String::from(gstring);
         assert_eq!(s, "foo");
@@ -1114,7 +1133,7 @@ mod tests {
     #[test]
     fn test_gstring_from_cstring() {
         let cstr = CString::new("foo").unwrap();
-        let gstring = GString::from(cstr);
+        let gstring = GString::try_from(cstr).unwrap();
         assert_eq!(gstring.as_str(), "foo");
         let foo: Box<str> = gstring.into();
         assert_eq!(foo.as_ref(), "foo");
@@ -1123,7 +1142,7 @@ mod tests {
     #[test]
     fn test_string_from_gstring_from_cstring() {
         let cstr = CString::new("foo").unwrap();
-        let gstring = GString::from(cstr);
+        let gstring = GString::try_from(cstr).unwrap();
         assert_eq!(gstring.as_str(), "foo");
         let s = String::from(gstring);
         assert_eq!(s, "foo");
@@ -1132,7 +1151,7 @@ mod tests {
     #[test]
     fn test_vec_u8_to_gstring() {
         let v: &[u8] = b"foo";
-        let s: GString = Vec::from(v).into();
+        let s: GString = Vec::from(v).try_into().unwrap();
         assert_eq!(s.as_str(), "foo");
     }
 
@@ -1165,11 +1184,11 @@ mod tests {
     fn test_hashmap() {
         use std::collections::HashMap;
 
-        let gstring = GString::from("foo");
+        let gstring = GString::try_from("foo").unwrap();
         assert_eq!(gstring.as_str(), "foo");
         let mut h: HashMap<GString, i32> = HashMap::new();
         h.insert(gstring, 42);
-        let gstring: GString = "foo".into();
+        let gstring: GString = "foo".try_into().unwrap();
         assert!(h.contains_key(&gstring));
     }
 }

glib/src/log.rs

@@ -859,8 +859,8 @@ macro_rules! g_printerr {
 ///         "MY_FIELD2" => "abc {}", "def";
 ///         // single argument can be a &str or a &[u8] or anything else satsfying AsRef<[u8]>
 ///         "MY_FIELD3" => CString::new("my string").unwrap().to_bytes();
-///         // field names can also be dynamic
-///         GString::from("MY_FIELD4") => b"a binary string".to_owned();
+///         // field names can also be dynamic, satisfying AsRef<GStr>
+///         GString::try_from("MY_FIELD4").unwrap() => b"a binary string".to_owned();
 ///         // the main log message goes in the MESSAGE field
 ///         "MESSAGE" => "test: {} {}", 1, 2, ;
 ///     }

glib/src/utils.rs

@@ -336,18 +336,18 @@ mod tests {
     fn test_filename_from_uri() {
         use crate::GString;
         use std::path::PathBuf;
-        let uri: GString = "file:///foo/bar.txt".into();
+        let uri: GString = "file:///foo/bar.txt".try_into().unwrap();
         if let Ok((filename, hostname)) = crate::filename_from_uri(&uri) {
             assert_eq!(filename, PathBuf::from(r"/foo/bar.txt"));
             assert_eq!(hostname, None);
         } else {
             unreachable!();
         }
 
-        let uri: GString = "file://host/foo/bar.txt".into();
+        let uri: GString = "file://host/foo/bar.txt".try_into().unwrap();
         if let Ok((filename, hostname)) = crate::filename_from_uri(&uri) {
             assert_eq!(filename, PathBuf::from(r"/foo/bar.txt"));
-            assert_eq!(hostname, Some(GString::from("host")));
+            assert_eq!(hostname, Some(GString::try_from("host").unwrap()));
         } else {
             unreachable!();
         }
@@ -358,19 +358,19 @@ mod tests {
         use crate::GString;
         assert_eq!(
             crate::uri_parse_scheme("foo://bar"),
-            Some(GString::from("foo"))
+            Some(GString::try_from("foo").unwrap())
         );
         assert_eq!(crate::uri_parse_scheme("foo"), None);
 
         let escaped = crate::uri_escape_string("&foo", None, true);
-        assert_eq!(escaped, GString::from("%26foo"));
+        assert_eq!(escaped, GString::try_from("%26foo").unwrap());
 
         let unescaped = crate::uri_unescape_string(escaped.as_str(), None);
-        assert_eq!(unescaped, Some(GString::from("&foo")));
+        assert_eq!(unescaped, Some(GString::try_from("&foo").unwrap()));
 
         assert_eq!(
             crate::uri_unescape_segment(Some("/foo"), None, None),
-            Some(GString::from("/foo"))
+            Some(GString::try_from("/foo").unwrap())
         );
         assert_eq!(crate::uri_unescape_segment(Some("/foo%"), None, None), None);
     }

glib/src/value.rs

@@ -1152,13 +1152,19 @@ mod tests {
         let v = vec!["123", "456"].to_value();
         assert_eq!(
             v.get::<Vec<GString>>(),
-            Ok(vec![GString::from("123"), GString::from("456")])
+            Ok(vec![
+                GString::try_from("123").unwrap(),
+                GString::try_from("456").unwrap()
+            ])
         );
 
         let v = vec![String::from("123"), String::from("456")].to_value();
         assert_eq!(
             v.get::<Vec<GString>>(),
-            Ok(vec![GString::from("123"), GString::from("456")])
+            Ok(vec![
+                GString::try_from("123").unwrap(),
+                GString::try_from("456").unwrap()
+            ])
         );
     }