Skip to content
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.

Sign up for GitHub

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

Jump to bottom

elf: Allow parsing SectionHeader from raw bytes #391

Merged
merged 2 commits into from
Mar 10, 2024
Merged

elf: Allow parsing SectionHeader from raw bytes #391

merged 2 commits into from
Mar 10, 2024

Conversation

lissyx
Copy link
Contributor

@lissyx lissyx commented Jan 25, 2024 •
edited
Loading

Fixes #390

m4b
m4b reviewed Feb 4, 2024
View reviewed changes
Copy link
Owner

@m4b m4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be good to review other names for this function, perhaps parse_lossy, i'm not sure if it does what the mach-o PR does, but otherwise this LGTM

src/elf/section_header.rs Show resolved Hide resolved
@lissyx
Copy link
Contributor Author

lissyx commented Feb 13, 2024

@m4b The lossy Mach-O PR seems to do a few different things, whereas here we just avoid checking the offset on the parse_lossy version. Their PR also introduce a parse_impl() so that might match better with what we do in fact ?

lissyx
lissyx commented Feb 13, 2024
View reviewed changes
tests/elf.rs Outdated Show resolved Hide resolved
@lissyx lissyx requested a review from m4b February 26, 2024 08:33
@lissyx
Copy link
Contributor Author

lissyx commented Mar 5, 2024

@m4b Gentle ping for re-reviewing ? Thanks !

@lissyx lissyx mentioned this pull request Mar 7, 2024
Closed
m4b
m4b reviewed Mar 10, 2024
View reviewed changes
Copy link
Owner

@m4b m4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just the name change (again, sorry!) then ready to go, thank you for your patience!

src/elf/section_header.rs Show resolved Hide resolved
tests/elf.rs Outdated Show resolved Hide resolved
src/elf/section_header.rs Outdated Show resolved Hide resolved
src/elf/section_header.rs Show resolved Hide resolved
@lissyx lissyx requested a review from m4b March 10, 2024 15:15
m4b
m4b approved these changes Mar 10, 2024
View reviewed changes
Copy link
Owner

@m4b m4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

@m4b m4b merged commit d73a80a into m4b:master Mar 10, 2024
6 checks passed
@m4b
Copy link
Owner

m4b commented Mar 10, 2024

release note: non-breaking

@m4b
Copy link
Owner

m4b commented Apr 27, 2024

released in 0.8.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orhun orhun orhun approved these changes

@m4b m4b m4b approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unable to parse ELF SectionHeader from raw bytes
3 participants
@lissyx @m4b @orhun