search

radareorg / r2pipe.rs

Rust crate for r2pipe
Other
45 stars 19 forks source link

Adds rop gadget retrieval #18

Closed chinmaydd closed 7 years ago

chinmaydd commented 7 years ago

I had made certain modifications for a side project which had to be abandoned. I think it's a useful contribution.

radare commented 7 years ago

i think r2pipe should be only in charge of the communication channel with r2, all those higher level abstractions should go into a separate crate. i can merge this if you want, but im not happy with the aa->aaa without option and having more high level apis, but its ok if it solves your current situation and we keep in the plan to separate those two crates.

chinmaydd commented 7 years ago

i think r2pipe should be only in charge of the communication channel with r2, all those higher level abstractions should go into a separate crate.

Do you mean we should write another library for a higher level r2 API for researchers to work with? That would imply using the underlying r2pipe to import structures and use them better.

im not happy with the aa->aaa without option and having more high level apis

I understand.

but its ok if it solves your current situation

It isn't solving much, actually. The project I was working on was abandoned. If you feel that the contribution is not helping much in this context, do close the request.

we keep in the plan to separate those two crates.

That does sound good. I am unsure of how the entire library stack will work though. Since the entirety of radeco-lib, rune and esil-rs depend on r2pipe providing them with useful structures. Maybe Sushant can put some more light on it?

radare commented 7 years ago

On 21 Mar 2017, at 19:28, Chinmay Deshpande notifications@github.com wrote: Do you mean we should write another library for a higher level r2 API for researchers to work with? That would imply using the underlying r2pipe to import structures and use them better.

yes im not happy with the aa->aaa without option and having more high level apis I understand.

some bins will take a lot of time with just one more a, but its good for others, so... but its ok if it solves your current situation It isn't solving much, actually. The project I was working on was abandoned. If you feel that the contribution is not helping much in this context, do close the request.

nah its ok to merge as i said, i just want to separate those two things :) we keep in the plan to separate those two crates. That does sound good. I am unsure of how the entire library stack will work though. Since the entirety of radeco-lib, rune and esil-rs depend on r2pipe providing them with useful structures. Maybe Sushant can put some more light on it?

are you in the mood to do that crate split? i think we can just rename this crate as r2pipe-frontend and then remove all the non-r2pipe-related things from r2pipe.rs http://r2pipe.rs/ and later make r2pipe-frontend use r2pipe.rs http://r2pipe.rs/. this would be the process to refactor that out.

let me know if you wants to work on that

Thanks!

chinmaydd commented 7 years ago

That sounds like a good idea.

are you in the mood to do that crate split? let me know if you wants to work on that

For sure! Not right now, I guess. I am working on my proposal for rune under (G)RSoc. Maybe I would be interested in taking it up as a side project sometime soon.

radare commented 7 years ago

Cool! I have absolutely no time but its been in my head since radeco

On 21 Mar 2017, at 20:49, Chinmay Deshpande notifications@github.com wrote:

That sounds like a good idea.

are you in the mood to do that crate split? let me know if you wants to work on that

For sure! Not right now, I guess. I am working on my proposal for rune under (G)RSoc. Maybe I would be interested in taking it up as a side project sometime soon.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

sushant94 commented 7 years ago

I'd like to have it here.

imo, most people would opt to use methods that would return them nice Rust structs (rather than raw json). Also, adding new features here does not mean that the users cannot access the vanilla r2pipe if they wish to (we aren't preventing/restricting this). So I see no harm.

radare commented 7 years ago

The harm is in modularity, reusability and reduce the size of the code to test and review and provide proper separation of logic

On 23 Mar 2017, at 03:03, Sushant Dinesh notifications@github.com wrote:

I'd like to have it here.

imo, most people would opt to use methods that would return them nice Rust structs (rather than raw json). Also, adding new features here does not mean that the users cannot access the vanilla r2pipe if they wish to (we aren't preventing/restricting this). So I see no harm.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

radare commented 7 years ago

ping

chinmaydd commented 7 years ago

@radare since there are more doubts about how to go about improving the r2pipe API, I am closing this PR for now. Also, this is not an urgent issue-fix anyways. Maybe we can have a detailed discussion offline and get the design sorted out.

radare commented 7 years ago

the r2pipe api shouldnt be improved in that direction. r2pipe is a simple communication protocol. im happy with having this in a separate module, so feel free to split the low level and high level logic into separate crates. see other implementations of r2pipe to see what’s the basic communication channels, all the rest of the logic behind commands and such must be moved in a separate place.

On 12 Jun 2017, at 16:49, Chinmay Deshpande notifications@github.com wrote:

@radare https://github.com/radare since there are more doubts about how to go about improving the r2pipe API, I am closing this PR for now. Also, this is not an urgent issue-fix anyways. Maybe we can have a detailed discussion offline and get the design sorted out.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/radare/r2pipe.rs/pull/18#issuecomment-307812705, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3-lvAlhlXp6D2V3Kl3JCl7N_MdUWUhks5sDVACgaJpZM4MkKq8.