Convert remaining modules to use Reader #214
Conversation
src/cfi.rs
Outdated
| @@ -766,7 +766,7 @@ impl Default for Augmentation { | |||
| } | |||
|
|
|||
| impl Augmentation { | |||
| fn parse<'aug, 'bases, 'input, Endian, Section>(augmentation_str: &'aug str, | |||
| fn parse<'aug, 'bases, 'input, Endian, Section>(augmentation_str: &'aug [u8], | |||
There was a problem hiding this comment.
Page 174, line 19 of the DWARF 5 standard (and earlier versions too) says that augmentation strings are encoded in UTF-8. Given that, why would we want to parse as &[u8]?
There was a problem hiding this comment.
So that we can trivially convert this to use the Reader trait. Since all of the characters that we understand currently are single-byte, it doesn't matter if we treat it as &[u8]. If we do need to handle multi-byte UTF-8 characters in future, we'll need to either potentially read the whole string into a Vec and then validate it as a UTF-8 string, or add a read_utf8_char method to the trait.
One thing we do lack from this approach is that we no longer validate that unknown augmentation strings are UTF-8. I don't see that as much of a problem though, since the user will handle this when they convert the Reader into a string.
src/cfi.rs
Outdated
| /// | ||
| /// It is the caller's responsibility to read the section and present it as | ||
| /// a `&[u8]` slice. That means using some ELF loader on Linux, a Mach-O | ||
| /// loader on OSX, etc. |
There was a problem hiding this comment.
Nitpick: not a &[u8] here, but a Reader
src/cfi.rs
Outdated
| /// | ||
| /// It is the caller's responsibility to read the section and present it as | ||
| /// a `&[u8]` slice. That means using some ELF loader on Linux, a Mach-O | ||
| /// loader on OSX, etc. |
| @@ -11,8 +11,7 @@ use parser::{Error, Result, Format, u64_to_offset}; | |||
| /// All read operations advance the section offset of the reader | |||
| /// unless specified otherwise. | |||
| /// | |||
| // TODO: Don't require Copy, Eq | |||
| pub trait Reader: Debug + Clone + Copy + PartialEq + Eq + Read { | |||
| pub trait Reader: Debug + Clone + Read { | |||
src/loc.rs
Outdated
| /// section. | ||
| /// | ||
| /// It is the caller's responsibility to read the `.debug_loc` section and | ||
| /// present it as a `&[u8]` slice. That means using some ELF loader on |
src/ranges.rs
Outdated
| /// section. | ||
| /// | ||
| /// It is the caller's responsibility to read the `.debug_ranges` section and | ||
| /// present it as a `&[u8]` slice. That means using some ELF loader on |
Ah, OK I see now that this was just a temporary thing until it became a LGTM, r+ with the nitpicks on copy-paste Thanks a third time! :) |
Also avoid
Copy/Eqrequirement by using offsets.Next step after this is to convert dwarfdump to use
Reader. Probably need to addto_vecandto_stringmethods to theReadertrait to allow this.