Skip to content

Commit 81cb0f7

Browse files
authored
Fix a portability problem in rosidl_runtime_rs::String (#219)
Previously, it was assumed by the $string_conversion_func, for instance, that the output of deref() would be an unsigned integer.
1 parent a351772 commit 81cb0f7

File tree

2 files changed

+18
-8
lines changed

2 files changed

+18
-8
lines changed

rosidl_runtime_rs/src/string.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ pub struct StringExceedsBoundsError {
110110

111111
// There is a lot of redundancy between String and WString, which this macro aims to reduce.
112112
macro_rules! string_impl {
113-
($string:ty, $char_type:ty, $string_conversion_func:ident, $init:ident, $fini:ident, $assignn:ident, $sequence_init:ident, $sequence_fini:ident, $sequence_copy:ident) => {
113+
($string:ty, $char_type:ty, $unsigned_char_type:ty, $string_conversion_func:ident, $init:ident, $fini:ident, $assignn:ident, $sequence_init:ident, $sequence_fini:ident, $sequence_copy:ident) => {
114114
#[link(name = "rosidl_runtime_c")]
115115
extern "C" {
116116
fn $init(s: *mut $string) -> bool;
@@ -159,21 +159,26 @@ macro_rules! string_impl {
159159
fn deref(&self) -> &Self::Target {
160160
// SAFETY: self.data points to self.size consecutive, initialized elements and
161161
// isn't modified externally.
162-
unsafe { std::slice::from_raw_parts(self.data as *const $char_type, self.size) }
162+
unsafe { std::slice::from_raw_parts(self.data, self.size) }
163163
}
164164
}
165165

166166
impl DerefMut for $string {
167167
fn deref_mut(&mut self) -> &mut Self::Target {
168168
// SAFETY: self.data points to self.size consecutive, initialized elements and
169169
// isn't modified externally.
170-
unsafe { std::slice::from_raw_parts_mut(self.data as *mut $char_type, self.size) }
170+
unsafe { std::slice::from_raw_parts_mut(self.data, self.size) }
171171
}
172172
}
173173

174174
impl Display for $string {
175175
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
176-
let converted = std::string::String::$string_conversion_func(self.deref());
176+
// SAFETY: Same as deref, but with an additional cast to the unsigned type.
177+
// See also https://users.rust-lang.org/t/how-to-convert-i8-to-u8/16308/11
178+
let u8_slice = unsafe {
179+
std::slice::from_raw_parts(self.data as *mut $unsigned_char_type, self.size)
180+
};
181+
let converted = std::string::String::$string_conversion_func(u8_slice);
177182
Display::fmt(&converted, f)
178183
}
179184
}
@@ -238,6 +243,7 @@ macro_rules! string_impl {
238243

239244
string_impl!(
240245
String,
246+
libc::c_char,
241247
u8,
242248
from_utf8_lossy,
243249
rosidl_runtime_c__String__init,
@@ -250,6 +256,7 @@ string_impl!(
250256
string_impl!(
251257
WString,
252258
libc::c_ushort,
259+
u16,
253260
from_utf16_lossy,
254261
rosidl_runtime_c__U16String__init,
255262
rosidl_runtime_c__U16String__fini,
@@ -321,7 +328,7 @@ impl<const N: usize> Debug for BoundedString<N> {
321328
}
322329

323330
impl<const N: usize> Deref for BoundedString<N> {
324-
type Target = [u8];
331+
type Target = [libc::c_char];
325332
fn deref(&self) -> &Self::Target {
326333
self.inner.deref()
327334
}

rosidl_runtime_rs/src/string/serde.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use serde::{de::Error, Deserialize, Deserializer, Serialize, Serializer};
2-
use std::ops::Deref;
32

43
use super::{BoundedString, BoundedWString, String, WString};
54

@@ -18,7 +17,9 @@ impl Serialize for String {
1817
S: Serializer,
1918
{
2019
// Not particularly efficient
21-
let s = std::string::String::from_utf8_lossy(self.deref());
20+
// SAFETY: See the Display implementation.
21+
let u8_slice = unsafe { std::slice::from_raw_parts(self.data as *mut u8, self.size) };
22+
let s = std::string::String::from_utf8_lossy(u8_slice);
2223
serializer.serialize_str(&s)
2324
}
2425
}
@@ -38,7 +39,9 @@ impl Serialize for WString {
3839
S: Serializer,
3940
{
4041
// Not particularly efficient
41-
let s = std::string::String::from_utf16_lossy(self.deref());
42+
// SAFETY: See the Display implementation.
43+
let u16_slice = unsafe { std::slice::from_raw_parts(self.data as *mut u16, self.size) };
44+
let s = std::string::String::from_utf16_lossy(u16_slice);
4245
serializer.serialize_str(&s)
4346
}
4447
}

0 commit comments

Comments
 (0)