New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolve symbols #46
Resolve symbols #46
Conversation
src/lib.rs
Outdated
@@ -1,5 +1,8 @@ | |||
//! Headcrab, a modern Rust debugging library. | |||
|
|||
#[macro_use] | |||
extern crate rental; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This crate seems to be archived & not maintained anymore and the author recommends to explore other solutions: https://github.com/jpernst/rental
As it seems, in this PR it's only used for safe borrowing of Mmap - I wonder if we can do the same with contained unsafe code? What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know it was archived.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have extracted the actual debuginfo parsing into ParsedDwarf::new
keeping Dwarf::new
as a thin wrapper that only handles the self-referential part. I have also written some unsafe code as replacement for rental. I am currently waiting on a review over at the t-lang/wg-unsafe-code-guidelines
stream of the rust-lang zulip.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
some unsafe code as replacement for rental
This can be submitted as a separate issue/PR, if it would be easier for you.
I'm going to review the rest of the code now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got a review, so I pushed it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! That's a great addition, thanks 👍
@@ -9,7 +9,7 @@ static BIN_PATH: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/testees/hell | |||
|
|||
#[cfg(all(target_os = "linux", target_arch = "x86_64"))] | |||
#[test] | |||
fn read_memory() -> Result<(), Box<dyn std::error::Error>> { | |||
fn read_regs() -> Result<(), Box<dyn std::error::Error>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch :)
Fixes #44
Based on #34