-
Notifications
You must be signed in to change notification settings - Fork 1
Open
Description
One of the very first scopes I tried was:
scope!("I/O", format!("class {}", file.name()));Suffice it to say it didn't work terribly well when evaluated exactly once:
Lines 189 to 196 in 2746d44
| ($group_name:expr, $scope_name:expr, $color:expr) => { | |
| { | |
| static mut TOKEN : u64 = 0; | |
| static INIT: std::sync::Once = std::sync::Once::new(); | |
| unsafe{ | |
| INIT.call_once(||{ | |
| let group = std::ffi::CString::new($group_name).unwrap(); | |
| let scope = std::ffi::CString::new($scope_name).unwrap(); |
Of course, this is a reasonable and common optimization for flamegraphing profilers, so I wasn't too terribly confused. That said, there's a a couple of options to help prevent this footgun. One would be to change the macro parameters to limit them to literals:
($group_name:literal, $scope_name:literal, $color:literal) => {Possibly combined with concat! to append '\0's at compile time instead of at runtime:
concat!($group_name, "\0")Or to encourage a static lifetime:
let group : &'static str = $group_name;
let scope : &'static str = $scope_name;
let group = std::ffi::CString::new(group).unwrap();
let scope = std::ffi::CString::new(scope).unwrap(); Which is still defeatable with Box::leak(Box::new(format!("look ma, {}!", "dynamic again"))), but at least you have to work for it, and the end user will hopefully question why they have to do such a thing ;)
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels