-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Kill the _end section flags and fix flag size #11349
Conversation
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.
LGTM
of course after fixing tests :D |
yep :P if anyone in the mood :P
… On 3 Sep 2018, at 10:11, Riccardo Schirone ***@***.***> wrote:
of course after fixing tests :D
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#11349 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA3-lgIn9NrZchYjYJKFXSlxqz5pmDJQks5uXOQlgaJpZM4WWa5q>.
|
Some people are going to miss those
Regarding the above, it might be better to have an |
that's true, though I think things should be displayed based on flag starting address + flag size, instead of having two flags as it is right now in master. |
might be tricky to implement efficiently but ok |
because i think its easier to improve the RNumCallback to handle $$flagname for this
i have a bunch of suggestions and ideas to improve the syntax for those numbers |
moved to $e{}
|
the _end flags introduce issues in disasm because it takes the _end instead of the next begin one and such, which use cases do people have for the _end flags? because i think with that $e{} is enough |
The e{} Sgtm |
Agree, we just need add this into the Sections chapter of r2book.
…On Thu, Sep 13, 2018, 5:06 PM Maijin ***@***.***> wrote:
The e{} Sgtm
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#11349 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAMZ_UMxl9UzQwDvXhxFofg3b6OZqOxCks5uah_9gaJpZM4WWa5q>
.
|
ok, rebasing and updating tests for the glory of the merge |
before adding this to the book i would prefer to discuss all the ?$? vars |
Fix #11348