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

Added feature gates (resolves issue #70) #123

Merged
merged 3 commits into from
Jul 6, 2021

Conversation

russells-crockpot
Copy link
Contributor

@russells-crockpot russells-crockpot commented Jul 3, 2021

This adds several feature gates to the core library. Specifically it added :

  • The emu feature, which will include the emu module (this is also automatically included when built with the test feature.
  • The flirt feature which adds the flirt module.
  • The rsrc and test modules will now only be included when testing (or running with the existing test feature).

Both of these features are enabled by default to maintain backwards compatibility.

Additionally, I removed superfluous features from other crates that the library depends on. For example, goblin ships with all of its features enabled by default. This includes support for things like elf files and mach files; neither of which are currently being used.

And one small, quick thing: I added the repository's URL to the Cargo.toml files to allow others to find the repo on crates.io

@russells-crockpot
Copy link
Contributor Author

Closing because I realized that issue #70 discusses some feature gates which I did not include (as I didn't want to do too much right away).
I'm going to implement issue #70 and then reopen this request. If, in the interim, there are any other feature-gates you'd like me to add, just let me know.

@russells-crockpot
Copy link
Contributor Author

Alright, I added the disassembler feature as requested in issue #70. I also renamed the emu feature to the more descriptive emulator

@russells-crockpot russells-crockpot changed the title Added feature gates Added feature gates (resolves issue #70) Jul 4, 2021
@williballenthin
Copy link
Owner

Thank you @russells-crockpot!

Great changes and I'm happy to merge them. In particular, I really appreciate that you figured out how to cut down the features used for goblin.

@williballenthin
Copy link
Owner

To be honest, I'm not aware that anyone else uses this project so I find it really neat that you've contributed. Are there any features that you expect to use from lancelot that I can give particular attention to?

@williballenthin williballenthin merged commit 23e6165 into williballenthin:master Jul 6, 2021
@russells-crockpot
Copy link
Contributor Author

Not really. I was planning on using is to simply parse PE data since goblin's API is lacking. Unfortunately, there are somethings in the code that I'm not a fan of, even if they make sense.

Specifically, the need to copy the data over to a vec when creating the PE and the need to reparse the PE everytime. At first I was wondering why, but then I realized that it was because doing it any other way would require a lifetime generic, which pyo doesn't support.

The only valid alternative that I can see would be to write my own PE parsing library, and because I'm a crazy person, I've decided to do just that (it's called peparse and it's not on crates.io yet). I'm specifically designing it so that it will be able to work with owned or unowned data. Once I'm done with it (or at least further along) we could see about possibly integrating it, but I just started on Sunday, so that might be a while.

@williballenthin
Copy link
Owner

and because I'm a crazy person, I've decided to do just that

Neat! I'll keep an eye on this project.

I was not super happy with the PE parsing setup, for the reasons you mentioned, but since there were a million other things to research, I decided it was good enough. For my use cases, the PE parsing operations are usually O(1) so even if its not super fast, there's a lot more time spent elsewhere.

Once peparse comes together, I'd be happy to work with you and try to use it for my projects like lancelot and others.

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