Skip to content

Commit 74cb2e1

Browse files
committed
refactor: introduced rstring_checked() and rstring_checked_or_empty() helper functions
After this change, the code always uses some helper function to convert from a Ruby type to a Rust string. The different flavours (checked, utf8 lossy, etc.) have been preserved. However, now we need to review the code carefully and decide whether each conversion handles UTF8 errors or NIL values correctly.
1 parent 9ef6466 commit 74cb2e1

File tree

1 file changed

+22
-23
lines changed
  • gems/codetracer-ruby-recorder/ext/native_tracer/src

1 file changed

+22
-23
lines changed

gems/codetracer-ruby-recorder/ext/native_tracer/src/lib.rs

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use std::{
77
os::raw::{c_char, c_int, c_void},
88
path::Path,
99
ptr,
10+
string::FromUtf8Error,
1011
};
1112

1213
use rb_sys::{
@@ -287,6 +288,21 @@ unsafe fn rstring_lossy(val: VALUE) -> String {
287288
String::from_utf8_lossy(slice).to_string()
288289
}
289290

291+
unsafe fn rstring_checked(val: VALUE) -> Result<String, FromUtf8Error> {
292+
let ptr = RSTRING_PTR(val);
293+
let len = RSTRING_LEN(val) as usize;
294+
let slice = std::slice::from_raw_parts(ptr as *const u8, len);
295+
String::from_utf8(slice.to_vec())
296+
}
297+
298+
unsafe fn rstring_checked_or_empty(val: VALUE) -> String {
299+
if NIL_P(val) {
300+
String::default()
301+
} else {
302+
rstring_checked(val).unwrap_or(String::default())
303+
}
304+
}
305+
290306
unsafe fn value_to_string(recorder: &Recorder, val: VALUE) -> String {
291307
if RB_TYPE_P(val, rb_sys::ruby_value_type::RUBY_T_STRING) {
292308
rstring_lossy(val)
@@ -598,9 +614,6 @@ unsafe fn record_event(tracer: &mut dyn TraceWriter, path: &str, line: i64, cont
598614
unsafe extern "C" fn initialize(self_val: VALUE, out_dir: VALUE, format: VALUE) -> VALUE {
599615
let recorder_ptr = get_recorder(self_val);
600616
let recorder = &mut *recorder_ptr;
601-
let ptr = RSTRING_PTR(out_dir) as *const u8;
602-
let len = RSTRING_LEN(out_dir) as usize;
603-
let slice = std::slice::from_raw_parts(ptr, len);
604617

605618
let fmt = if !NIL_P(format) && RB_SYMBOL_P(format) {
606619
match cstr_to_string(rb_id2name(rb_sym2id(format)))
@@ -616,9 +629,9 @@ unsafe extern "C" fn initialize(self_val: VALUE, out_dir: VALUE, format: VALUE)
616629
runtime_tracing::TraceEventsFileFormat::Json
617630
};
618631

619-
match std::str::from_utf8(slice) {
632+
match rstring_checked(out_dir) {
620633
Ok(path_str) => {
621-
match begin_trace(Path::new(path_str), fmt) {
634+
match begin_trace(Path::new(&path_str), fmt) {
622635
Ok(t) => {
623636
recorder.tracer = t;
624637
// pre-register common types to match the pure Ruby tracer
@@ -709,16 +722,10 @@ unsafe extern "C" fn record_event_api(
709722
content: VALUE,
710723
) -> VALUE {
711724
let recorder = &mut *get_recorder(self_val);
712-
let path_slice = if NIL_P(path) {
713-
""
714-
} else {
715-
let ptr = RSTRING_PTR(path);
716-
let len = RSTRING_LEN(path) as usize;
717-
std::str::from_utf8(std::slice::from_raw_parts(ptr as *const u8, len)).unwrap_or("")
718-
};
725+
let path_string = rstring_checked_or_empty(path);
719726
let line_num = rb_num2long(line) as i64;
720727
let content_str = value_to_string(recorder, content);
721-
record_event(&mut *recorder.tracer, path_slice, line_num, content_str);
728+
record_event(&mut *recorder.tracer, &path_string, line_num, content_str);
722729
Qnil.into()
723730
}
724731

@@ -742,17 +749,9 @@ unsafe extern "C" fn event_hook_raw(data: VALUE, arg: *mut rb_trace_arg_t) {
742749
let ev: rb_event_flag_t = rb_tracearg_event_flag(arg);
743750
let path_val = rb_tracearg_path(arg);
744751
let line_val = rb_tracearg_lineno(arg);
745-
let path = if NIL_P(path_val) {
746-
""
747-
} else {
748-
let path_bytes = std::slice::from_raw_parts(
749-
RSTRING_PTR(path_val) as *const u8,
750-
RSTRING_LEN(path_val) as usize,
751-
);
752-
std::str::from_utf8(path_bytes).unwrap_or("")
753-
};
752+
let path = rstring_checked_or_empty(path_val);
754753
let line = rb_num2long(line_val) as i64;
755-
if should_ignore_path(path) {
754+
if should_ignore_path(&path) {
756755
return;
757756
}
758757

0 commit comments

Comments
 (0)