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.

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

Adding option 'Host' to BinaryFormat enum + new exampel #661

Merged
merged 10 commits into from
Apr 4, 2024
Merged

Adding option 'Host' to BinaryFormat enum + new exampel #661

merged 10 commits into from
Apr 4, 2024

Conversation

Cr0a3
Copy link
Contributor

@Cr0a3 Cr0a3 commented Apr 2, 2024

Hi,
I added the enum-varient Host to the BinaryFormat enum, which automaticly maps under Windows to Coff on MacOS to Macho and on other platforms to Elf.
I also added the translated faerie example (thanks @philipc for the code while he was very nicely helping me out), because i think the exampels which are currently in place, are hard to understand as someone (like me) comming from faerie

Bye

Copy link
Contributor

@philipc philipc left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I'd like a slightly different API for the binary format, see the comments.

Also, please don't add the example in this PR. It was just a rough conversion of the faerie code. I'll add a better example in a different PR.

src/common.rs Outdated Show resolved Hide resolved
src/write/mod.rs Outdated
pub fn new(mut format: BinaryFormat, architecture: Architecture, endian: Endianness) -> Object<'a> {
if format == BinaryFormat::Host {
format = format.get();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This change can be deleted after Host is replaced with native_object.

src/common.rs Outdated Show resolved Hide resolved
src/common.rs Outdated Show resolved Hide resolved
Cr0a3 and others added 4 commits April 3, 2024 09:25
@Cr0a3
Copy link
Contributor Author

Cr0a3 commented Apr 3, 2024

Ok i integrated your changes but i renamed native_object to host.
And I also removed (like you requested ) your translated faerie example.
Should i then also remove then the empty example directory?

@philipc
Copy link
Contributor

philipc commented Apr 3, 2024

Yes please remove the directory too.

Also keep the name as native_object, since that is consistent with the existing name for read::NativeFile, and because it is specifically for relocatable object files, not executable files (which have a different format for Windows).

@Cr0a3
Copy link
Contributor Author

Cr0a3 commented Apr 3, 2024

I fixed it

src/common.rs Outdated Show resolved Hide resolved
@philipc philipc merged commit bbd6aa8 into gimli-rs:master Apr 4, 2024
10 checks passed
mcbegamerxx954 pushed a commit to mcbegamerxx954/object that referenced this pull request Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants