Skip to content

Commit 9699f4b

Browse files
committed
fix: Clean up some of the cruft generated by Codex
- Remove the use of Mutex (Ruby already provides a global lock) - Remove the pointless init function - Add error handling
1 parent eff8fbe commit 9699f4b

File tree

2 files changed

+18
-25
lines changed

2 files changed

+18
-25
lines changed

examples/selective_tracing.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,4 @@
1111
puts 'this will be traced'
1212
recorder.disable_tracing
1313
puts 'tracing disabled'
14-
recorder.flush_trace(ENV['CODETRACER_RUBY_RECORDER_OUT_DIR'] || Dir.pwd)
14+
recorder.flush_trace(Dir.pwd)

gems/native-tracer/ext/native_tracer/src/lib.rs

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use std::{
66
os::raw::{c_char, c_void},
77
path::Path,
88
ptr,
9-
sync::Mutex,
109
};
1110

1211
use rb_sys::{
@@ -56,7 +55,7 @@ extern "C" {
5655
}
5756

5857
struct Recorder {
59-
tracer: Mutex<Tracer>,
58+
tracer: Tracer,
6059
active: bool,
6160
}
6261

@@ -86,16 +85,11 @@ unsafe fn get_recorder(obj: VALUE) -> *mut Recorder {
8685
}
8786

8887
unsafe extern "C" fn ruby_recorder_alloc(klass: VALUE) -> VALUE {
89-
let recorder = Box::new(Recorder { tracer: Mutex::new(Tracer::new("ruby", &vec![])), active: false });
88+
let recorder = Box::new(Recorder { tracer: Tracer::new("ruby", &vec![]), active: false });
9089
let ty = std::ptr::addr_of!(RECORDER_TYPE) as *const rb_data_type_t;
9190
rb_data_typed_object_wrap(klass, Box::into_raw(recorder) as *mut c_void, ty)
9291
}
9392

94-
unsafe extern "C" fn ruby_recorder_initialize(_self: VALUE) -> VALUE {
95-
// nothing special for now
96-
rb_sys::Qnil.into()
97-
}
98-
9993
unsafe extern "C" fn enable_tracing(self_val: VALUE) -> VALUE {
10094
let recorder = &mut *get_recorder(self_val);
10195
if !recorder.active {
@@ -118,14 +112,15 @@ unsafe extern "C" fn disable_tracing(self_val: VALUE) -> VALUE {
118112
rb_sys::Qnil.into()
119113
}
120114

121-
fn flush_to_dir(tracer: &Tracer, dir: &Path) {
122-
let _ = std::fs::create_dir_all(dir);
115+
fn flush_to_dir(tracer: &Tracer, dir: &Path) -> std::io::Result<()> {
116+
std::fs::create_dir_all(dir)?;
123117
let events = dir.join("trace.json");
124118
let metadata = dir.join("trace_metadata.json");
125119
let paths = dir.join("trace_paths.json");
126-
let _ = tracer.store_trace_events(&events);
127-
let _ = tracer.store_trace_metadata(&metadata);
128-
let _ = tracer.store_trace_paths(&paths);
120+
tracer.store_trace_events(&events)?;
121+
tracer.store_trace_metadata(&metadata)?;
122+
tracer.store_trace_paths(&paths)?;
123+
Ok(())
129124
}
130125

131126
unsafe extern "C" fn flush_trace(self_val: VALUE, out_dir: VALUE) -> VALUE {
@@ -134,14 +129,16 @@ unsafe extern "C" fn flush_trace(self_val: VALUE, out_dir: VALUE) -> VALUE {
134129
let ptr = RSTRING_PTR(out_dir) as *const u8;
135130
let len = RSTRING_LEN(out_dir) as usize;
136131
let slice = std::slice::from_raw_parts(ptr, len);
137-
if let Ok(path_str) = std::str::from_utf8(slice) {
138-
if let Ok(t) = recorder.tracer.lock() {
139-
flush_to_dir(&t, Path::new(path_str));
132+
133+
match std::str::from_utf8(slice) {
134+
Ok(path_str) => {
135+
if let Err(e) = flush_to_dir(&recorder.tracer, Path::new(path_str)) {
136+
rb_raise(rb_eIOError, b"Failed to flush trace: %s\0".as_ptr() as *const c_char, e.to_string().as_ptr() as *const c_char);
137+
}
140138
}
139+
Err(e) => rb_raise(rb_eIOError, b"Invalid UTF-8 in path: %s\0".as_ptr() as *const c_char, e.to_string().as_ptr() as *const c_char),
141140
}
142-
drop(Box::from_raw(recorder_ptr));
143-
let rdata = self_val as *mut RTypedData;
144-
(*rdata).data = ptr::null_mut();
141+
145142
rb_sys::Qnil.into()
146143
}
147144

@@ -172,9 +169,7 @@ unsafe extern "C" fn event_hook_raw(data: VALUE, arg: *mut rb_trace_arg_t) {
172169

173170
if !path_ptr.is_null() {
174171
if let Ok(path) = CStr::from_ptr(path_ptr).to_str() {
175-
if let Ok(mut t) = recorder.tracer.lock() {
176-
t.register_step(Path::new(path), Line(line));
177-
}
172+
recorder.tracer.register_step(Path::new(path), Line(line));
178173
}
179174
}
180175
}
@@ -184,11 +179,9 @@ pub extern "C" fn Init_codetracer_ruby_recorder() {
184179
unsafe {
185180
let class = rb_define_class(b"RubyRecorder\0".as_ptr() as *const c_char, rb_cObject);
186181
rb_define_alloc_func(class, Some(ruby_recorder_alloc));
187-
let init_cb: unsafe extern "C" fn(VALUE) -> VALUE = ruby_recorder_initialize;
188182
let enable_cb: unsafe extern "C" fn(VALUE) -> VALUE = enable_tracing;
189183
let disable_cb: unsafe extern "C" fn(VALUE) -> VALUE = disable_tracing;
190184
let flush_cb: unsafe extern "C" fn(VALUE, VALUE) -> VALUE = flush_trace;
191-
rb_define_method(class, b"initialize\0".as_ptr() as *const c_char, Some(transmute(init_cb)), 0);
192185
rb_define_method(class, b"enable_tracing\0".as_ptr() as *const c_char, Some(transmute(enable_cb)), 0);
193186
rb_define_method(class, b"disable_tracing\0".as_ptr() as *const c_char, Some(transmute(disable_cb)), 0);
194187
rb_define_method(class, b"flush_trace\0".as_ptr() as *const c_char, Some(transmute(flush_cb)), 1);

0 commit comments

Comments
 (0)