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

Return the disassembled code #3

Closed
wants to merge 1 commit into from

Conversation

badboy
Copy link
Collaborator

@badboy badboy commented Jan 16, 2017

Instead of printing, return the array of instructions. This lets the user decide how and where to print it.

@qmonnet
Copy link
Owner

qmonnet commented Jan 16, 2017

Thanks!
But if we do that, we can as well delete the function, because the only thing it does is calling to_insn_vec() which is also public and printing its content (I'm realizing the checks on length at its beginning are redundant with those in to_insn_vec() and should be removed).

So with the current API, if the user wants to handle the vector of instructions they only have to call to_insn_vec(&prog) instead of disassemble(&prog) (see the example to_json.rs, this is exactly what happens). Do you think we should change that? The name of the functions maybe?

@badboy
Copy link
Collaborator Author

badboy commented Jan 16, 2017

Oh indeed, in that case we could just rename to_insn_vec to disassemble to make it obvious and printing can be handled in code using this API

@qmonnet
Copy link
Owner

qmonnet commented Jan 16, 2017

Would be fine for me. Just a note before we do that: I also added a to_insn_vec() function to the ebpf module, and it does exactly the same as its counterpart in disassembler module, on a lower level (not grouping values for 16-bytes long lddw, not adding name and description). As they do practically the same I thought it was helpful to have the same name, but in different modules. I wrote it in case some users would like to print the 8-bytes insns one by one, or something like that.

So if we rename disassembler::to_insn_vec() into disassembler::disassemble, do you have a suggestion for renaming the one in ebpf module? Or for what to do about it?

@badboy
Copy link
Collaborator Author

badboy commented Jan 16, 2017

Good question. I have to re-read the code for that first.

@badboy badboy closed this Jul 6, 2021
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.

None yet

2 participants