-
Notifications
You must be signed in to change notification settings - Fork 7
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
Rewrite system_calls.pl in Ruby #27
Conversation
Will certainly be (at least somewhat) cleaned up before merging in, since the code quality is currently a bit frightening...
…to what the .pl file used to create. Only three files left to verify... :)
…antically equivalent. There are a few whitespace adjustments that will need to be done though. Coming up next.
create_wrapper_c system_calls | ||
create_include_storm_system_calls_h system_calls | ||
create_system_calls_auto_c system_calls | ||
create_include_storm_ia32_wrapper_h system_calls |
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 could arguably be done even cleaner and nicer using ERB syntax. I would be very glad to incorporate changes in that direction; PR:s are welcome. However, for now I just did a pretty much 1-to-1 conversion (but changing to more Rubish syntax (.each
etc), and using a hash instead of an plan array etc). But changing to ERB would be even nicer. I consider this "good enough" for now though.
|
||
"popl %edi\n" | ||
"popl %esi\n" | ||
"popl %ebp\n" |
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.
The indentation was "off by one" in the output that the old script used to create, so I fixed that while I was at it.
@CaspecoHenrik and/or @johannesl, are you happy with merging? I've tested this and I get the same behavior as before (i.e. the bug noted in #21. 😉) So I think we're safe from a "functionality equivalence" POV) |
Nice! Can you confirm this works on the standard Ruby version in the latest OS X? Which is: |
…eing used in the Vagrant-provided box.
…ed whenever the system_calls.rb file is modified.
Thanks. 😄
It was actually the one I used when developing it, so yes, it works. I also fixed now so that it works with Ruby 1.9.3 (so it can run in the Vagrant-provided VM, for instance). Taking your comment as approval, so merging now. 😄 Thanks! |
…l-in-ruby Rewrite system_calls.pl in ruby
The idea is to get rid of an unnecessary Perl dependency. Since we already rely on Ruby & Rake for the build process, we might as well use it here. It is after all a very nice dynamic/scripting language.