Skip to content

Commit 9e2ca9c

Browse files
jf2048jf2048
committed
glib: Use TryFrom for GString conversions
1 parent db614bb commit 9e2ca9c

File tree

6 files changed

+89
-63
lines changed

6 files changed

+89
-63
lines changed

‎gio/src/file.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ impl<O: IsA<File>> FileExtManual for O {
237237
+ 'static,
238238
>,
239239
> {
240-
let etag = etag.map(glib::GString::from);
240+
let etag = etag.map(|e| glib::GString::try_from(e).unwrap());
241241
Box::pin(crate::GioFuture::new(
242242
self,
243243
move |obj, cancellable, send| {

‎glib/src/gstring.rs

Lines changed: 69 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ impl GStr {
3333
pub fn from_str_with_nul(s: &str) -> Result<&Self, std::ffi::FromBytesWithNulError> {
3434
let bytes = s.as_bytes();
3535
CStr::from_bytes_with_nul(bytes)?;
36-
Ok(unsafe { Self::from_bytes_with_nul_unchecked(bytes) })
36+
Ok(unsafe { Self::from_utf8_with_nul_unchecked(bytes) })
3737
}
3838
// rustdoc-stripper-ignore-next
3939
/// Unsafely creates a GLib string wrapper from a byte slice.
@@ -42,7 +42,7 @@ impl GStr {
4242
/// sanity checks. The provided slice **must** be valid UTF-8, nul-terminated and not contain
4343
/// any interior nul bytes.
4444
#[inline]
45-
pub const unsafe fn from_bytes_with_nul_unchecked(bytes: &[u8]) -> &Self {
45+
pub const unsafe fn from_utf8_with_nul_unchecked(bytes: &[u8]) -> &Self {
4646
debug_assert!(!bytes.is_empty() && bytes[bytes.len() - 1] == 0);
4747
mem::transmute(bytes)
4848
}
@@ -53,7 +53,7 @@ impl GStr {
5353
#[inline]
5454
pub unsafe fn from_ptr<'a>(ptr: *const c_char) -> &'a Self {
5555
let cstr = CStr::from_ptr(ptr);
56-
Self::from_bytes_with_nul_unchecked(cstr.to_bytes_with_nul())
56+
Self::from_utf8_with_nul_unchecked(cstr.to_bytes_with_nul())
5757
}
5858
// rustdoc-stripper-ignore-next
5959
/// Converts this GLib string to a byte slice containing the trailing 0 byte.
@@ -126,7 +126,7 @@ impl GStr {
126126
#[macro_export]
127127
macro_rules! gstr {
128128
($s:literal) => {
129-
unsafe { $crate::GStr::from_bytes_with_nul_unchecked($crate::cstr_bytes!($s)) }
129+
unsafe { $crate::GStr::from_utf8_with_nul_unchecked($crate::cstr_bytes!($s)) }
130130
};
131131
}
132132

@@ -142,7 +142,7 @@ impl<'a> TryFrom<&'a CStr> for &'a GStr {
142142
#[inline]
143143
fn try_from(s: &'a CStr) -> Result<Self, Self::Error> {
144144
s.to_str()?;
145-
Ok(unsafe { GStr::from_bytes_with_nul_unchecked(s.to_bytes_with_nul()) })
145+
Ok(unsafe { GStr::from_utf8_with_nul_unchecked(s.to_bytes_with_nul()) })
146146
}
147147
}
148148

@@ -270,7 +270,7 @@ unsafe impl<'a> crate::value::FromValue<'a> for &'a GStr {
270270
let ptr = gobject_ffi::g_value_get_string(value.to_glib_none().0);
271271
let cstr = CStr::from_ptr(ptr);
272272
assert!(cstr.to_str().is_ok());
273-
GStr::from_bytes_with_nul_unchecked(cstr.to_bytes_with_nul())
273+
GStr::from_utf8_with_nul_unchecked(cstr.to_bytes_with_nul())
274274
}
275275
}
276276

@@ -388,7 +388,7 @@ impl GString {
388388
slice::from_raw_parts(ptr.as_ptr() as *const _, len + 1)
389389
},
390390
};
391-
unsafe { GStr::from_bytes_with_nul_unchecked(bytes) }
391+
unsafe { GStr::from_utf8_with_nul_unchecked(bytes) }
392392
}
393393

394394
// rustdoc-stripper-ignore-next
@@ -426,7 +426,7 @@ impl GString {
426426

427427
impl Clone for GString {
428428
fn clone(&self) -> GString {
429-
self.as_str().into()
429+
self.as_gstr().to_owned()
430430
}
431431
}
432432

@@ -651,23 +651,23 @@ impl From<GString> for Box<str> {
651651
}
652652
}
653653

654-
impl From<String> for GString {
654+
impl TryFrom<String> for GString {
655+
type Error = std::ffi::NulError;
655656
#[inline]
656-
fn from(s: String) -> Self {
657+
fn try_from(s: String) -> Result<Self, Self::Error> {
657658
// Moves the content of the String
658-
unsafe {
659-
// No check for valid UTF-8 here
660-
let cstr = CString::from_vec_unchecked(s.into_bytes());
661-
GString(Inner::Native(Some(cstr)))
662-
}
659+
// Check for interior nul bytes
660+
let cstr = CString::new(s.into_bytes())?;
661+
Ok(GString(Inner::Native(Some(cstr))))
663662
}
664663
}
665664

666-
impl From<Box<str>> for GString {
665+
impl TryFrom<Box<str>> for GString {
666+
type Error = std::ffi::NulError;
667667
#[inline]
668-
fn from(s: Box<str>) -> Self {
668+
fn try_from(s: Box<str>) -> Result<Self, Self::Error> {
669669
// Moves the content of the String
670-
s.into_string().into()
670+
s.into_string().try_into()
671671
}
672672
}
673673

@@ -678,50 +678,69 @@ impl From<&GStr> for GString {
678678
}
679679
}
680680

681-
impl From<&str> for GString {
681+
impl TryFrom<&str> for GString {
682+
type Error = std::ffi::FromBytesWithNulError;
682683
#[inline]
683-
fn from(s: &str) -> Self {
684+
fn try_from(s: &str) -> Result<Self, Self::Error> {
684685
// Allocates with the GLib allocator
685686
unsafe {
686687
// No check for valid UTF-8 here
687688
let copy = ffi::g_malloc(s.len() + 1) as *mut c_char;
688689
ptr::copy_nonoverlapping(s.as_ptr() as *const c_char, copy, s.len() + 1);
689690
ptr::write(copy.add(s.len()), 0);
690691

691-
GString(Inner::Foreign {
692+
// Check for interior nul bytes
693+
let check = std::ffi::CStr::from_bytes_with_nul(std::slice::from_raw_parts(
694+
copy as *const _,
695+
s.len() + 1,
696+
));
697+
if let Err(err) = check {
698+
ffi::g_free(copy as *mut _);
699+
return Err(err);
700+
}
701+
702+
Ok(GString(Inner::Foreign {
692703
ptr: ptr::NonNull::new_unchecked(copy),
693704
len: s.len(),
694-
})
705+
}))
695706
}
696707
}
697708
}
698709

699-
impl From<Vec<u8>> for GString {
710+
#[derive(thiserror::Error, Debug)]
711+
pub enum GStringError {
712+
#[error("invalid UTF-8")]
713+
Utf8(#[from] std::str::Utf8Error),
714+
#[error("interior nul bytes")]
715+
Nul(#[from] std::ffi::NulError),
716+
}
717+
718+
impl TryFrom<Vec<u8>> for GString {
719+
type Error = GStringError;
700720
#[inline]
701-
fn from(s: Vec<u8>) -> Self {
721+
fn try_from(s: Vec<u8>) -> Result<Self, Self::Error> {
702722
// Moves the content of the Vec
703-
// Also check if it's valid UTF-8
704-
let cstring = CString::new(s).expect("CString::new failed");
705-
cstring.into()
723+
// Also check if it's valid UTF-8 and has no interior nuls
724+
let cstring = CString::new(s)?;
725+
Ok(cstring.try_into()?)
706726
}
707727
}
708728

709-
impl From<CString> for GString {
710-
#[inline]
711-
fn from(s: CString) -> Self {
729+
impl TryFrom<CString> for GString {
730+
type Error = std::str::Utf8Error;
731+
fn try_from(s: CString) -> Result<Self, Self::Error> {
712732
// Moves the content of the CString
713733
// Also check if it's valid UTF-8
714-
assert!(s.to_str().is_ok());
715-
Self(Inner::Native(Some(s)))
734+
s.to_str()?;
735+
Ok(Self(Inner::Native(Some(s))))
716736
}
717737
}
718738

719-
impl From<&CStr> for GString {
739+
impl TryFrom<&CStr> for GString {
740+
type Error = std::str::Utf8Error;
720741
#[inline]
721-
fn from(c: &CStr) -> Self {
722-
// Creates a copy with the GLib allocator
723-
// Also check if it's valid UTF-8
724-
c.to_str().unwrap().into()
742+
fn try_from(c: &CStr) -> Result<Self, Self::Error> {
743+
c.to_owned().try_into()
725744
}
726745
}
727746

@@ -772,7 +791,7 @@ impl FromGlibPtrNone<*const u8> for GString {
772791
assert!(!ptr.is_null());
773792
let cstr = CStr::from_ptr(ptr as *const _);
774793
// Also check if it's valid UTF-8
775-
cstr.to_str().unwrap().into()
794+
cstr.try_into().unwrap()
776795
}
777796
}
778797

@@ -904,16 +923,16 @@ impl<'a> ToGlibPtr<'a, *mut i8> for GString {
904923
impl<'a> FromGlibContainer<*const c_char, *const i8> for GString {
905924
unsafe fn from_glib_none_num(ptr: *const i8, num: usize) -> Self {
906925
if num == 0 || ptr.is_null() {
907-
return Self::from("");
926+
return Self::try_from("").unwrap();
908927
}
909928
let slice = slice::from_raw_parts(ptr as *const u8, num);
910929
// Also check if it's valid UTF-8
911-
std::str::from_utf8(slice).unwrap().into()
930+
std::str::from_utf8(slice).unwrap().try_into().unwrap()
912931
}
913932

914933
unsafe fn from_glib_container_num(ptr: *const i8, num: usize) -> Self {
915934
if num == 0 || ptr.is_null() {
916-
return Self::from("");
935+
return Self::try_from("").unwrap();
917936
}
918937

919938
// Check if it's valid UTF-8
@@ -928,7 +947,7 @@ impl<'a> FromGlibContainer<*const c_char, *const i8> for GString {
928947

929948
unsafe fn from_glib_full_num(ptr: *const i8, num: usize) -> Self {
930949
if num == 0 || ptr.is_null() {
931-
return Self::from("");
950+
return Self::try_from("").unwrap();
932951
}
933952

934953
// Check if it's valid UTF-8
@@ -1007,7 +1026,7 @@ unsafe impl<'a> crate::value::FromValue<'a> for GString {
10071026
type Checker = crate::value::GenericValueTypeOrNoneChecker<Self>;
10081027

10091028
unsafe fn from_value(value: &'a crate::Value) -> Self {
1010-
Self::from(<&str>::from_value(value))
1029+
Self::try_from(<&str>::from_value(value)).unwrap()
10111030
}
10121031
}
10131032

@@ -1097,15 +1116,15 @@ mod tests {
10971116

10981117
#[test]
10991118
fn test_gstring_from_str() {
1100-
let gstring: GString = "foo".into();
1119+
let gstring: GString = "foo".try_into().unwrap();
11011120
assert_eq!(gstring.as_str(), "foo");
11021121
let foo: Box<str> = gstring.into();
11031122
assert_eq!(foo.as_ref(), "foo");
11041123
}
11051124

11061125
#[test]
11071126
fn test_string_from_gstring() {
1108-
let gstring = GString::from("foo");
1127+
let gstring = GString::try_from("foo").unwrap();
11091128
assert_eq!(gstring.as_str(), "foo");
11101129
let s = String::from(gstring);
11111130
assert_eq!(s, "foo");
@@ -1114,7 +1133,7 @@ mod tests {
11141133
#[test]
11151134
fn test_gstring_from_cstring() {
11161135
let cstr = CString::new("foo").unwrap();
1117-
let gstring = GString::from(cstr);
1136+
let gstring = GString::try_from(cstr).unwrap();
11181137
assert_eq!(gstring.as_str(), "foo");
11191138
let foo: Box<str> = gstring.into();
11201139
assert_eq!(foo.as_ref(), "foo");
@@ -1123,7 +1142,7 @@ mod tests {
11231142
#[test]
11241143
fn test_string_from_gstring_from_cstring() {
11251144
let cstr = CString::new("foo").unwrap();
1126-
let gstring = GString::from(cstr);
1145+
let gstring = GString::try_from(cstr).unwrap();
11271146
assert_eq!(gstring.as_str(), "foo");
11281147
let s = String::from(gstring);
11291148
assert_eq!(s, "foo");
@@ -1132,7 +1151,7 @@ mod tests {
11321151
#[test]
11331152
fn test_vec_u8_to_gstring() {
11341153
let v: &[u8] = b"foo";
1135-
let s: GString = Vec::from(v).into();
1154+
let s: GString = Vec::from(v).try_into().unwrap();
11361155
assert_eq!(s.as_str(), "foo");
11371156
}
11381157

@@ -1165,11 +1184,11 @@ mod tests {
11651184
fn test_hashmap() {
11661185
use std::collections::HashMap;
11671186

1168-
let gstring = GString::from("foo");
1187+
let gstring = GString::try_from("foo").unwrap();
11691188
assert_eq!(gstring.as_str(), "foo");
11701189
let mut h: HashMap<GString, i32> = HashMap::new();
11711190
h.insert(gstring, 42);
1172-
let gstring: GString = "foo".into();
1191+
let gstring: GString = "foo".try_into().unwrap();
11731192
assert!(h.contains_key(&gstring));
11741193
}
11751194
}

‎glib/src/log.rs

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

‎glib/src/utils.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -336,18 +336,18 @@ mod tests {
336336
fn test_filename_from_uri() {
337337
use crate::GString;
338338
use std::path::PathBuf;
339-
let uri: GString = "file:///foo/bar.txt".into();
339+
let uri: GString = "file:///foo/bar.txt".try_into().unwrap();
340340
if let Ok((filename, hostname)) = crate::filename_from_uri(&uri) {
341341
assert_eq!(filename, PathBuf::from(r"/foo/bar.txt"));
342342
assert_eq!(hostname, None);
343343
} else {
344344
unreachable!();
345345
}
346346

347-
let uri: GString = "file://host/foo/bar.txt".into();
347+
let uri: GString = "file://host/foo/bar.txt".try_into().unwrap();
348348
if let Ok((filename, hostname)) = crate::filename_from_uri(&uri) {
349349
assert_eq!(filename, PathBuf::from(r"/foo/bar.txt"));
350-
assert_eq!(hostname, Some(GString::from("host")));
350+
assert_eq!(hostname, Some(GString::try_from("host").unwrap()));
351351
} else {
352352
unreachable!();
353353
}
@@ -358,19 +358,19 @@ mod tests {
358358
use crate::GString;
359359
assert_eq!(
360360
crate::uri_parse_scheme("foo://bar"),
361-
Some(GString::from("foo"))
361+
Some(GString::try_from("foo").unwrap())
362362
);
363363
assert_eq!(crate::uri_parse_scheme("foo"), None);
364364

365365
let escaped = crate::uri_escape_string("&foo", None, true);
366-
assert_eq!(escaped, GString::from("%26foo"));
366+
assert_eq!(escaped, GString::try_from("%26foo").unwrap());
367367

368368
let unescaped = crate::uri_unescape_string(escaped.as_str(), None);
369-
assert_eq!(unescaped, Some(GString::from("&foo")));
369+
assert_eq!(unescaped, Some(GString::try_from("&foo").unwrap()));
370370

371371
assert_eq!(
372372
crate::uri_unescape_segment(Some("/foo"), None, None),
373-
Some(GString::from("/foo"))
373+
Some(GString::try_from("/foo").unwrap())
374374
);
375375
assert_eq!(crate::uri_unescape_segment(Some("/foo%"), None, None), None);
376376
}

‎glib/src/value.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1152,13 +1152,19 @@ mod tests {
11521152
let v = vec!["123", "456"].to_value();
11531153
assert_eq!(
11541154
v.get::<Vec<GString>>(),
1155-
Ok(vec![GString::from("123"), GString::from("456")])
1155+
Ok(vec![
1156+
GString::try_from("123").unwrap(),
1157+
GString::try_from("456").unwrap()
1158+
])
11561159
);
11571160

11581161
let v = vec![String::from("123"), String::from("456")].to_value();
11591162
assert_eq!(
11601163
v.get::<Vec<GString>>(),
1161-
Ok(vec![GString::from("123"), GString::from("456")])
1164+
Ok(vec![
1165+
GString::try_from("123").unwrap(),
1166+
GString::try_from("456").unwrap()
1167+
])
11621168
);
11631169
}
11641170

0 commit comments

Comments
 (0)