Another attempt to do EsilHooks ##esil #21939
Conversation
condret
force-pushed
the
esil_hooks2
branch
6 times, most recently
from
6aae0a8 to
87c6114
Compare
There was a problem hiding this comment.
i like the way it works, but i think this introduces so many methods when we can make it much simpler by using a single function that takes operation as argument.
It's also very similar to how REvent works. maybe better to re-evaluate to improve this api that we dont use yet because we can have multiple event handlers around and that will be important for ranal too.
What do you think?
| @@ -4011,10 +4018,16 @@ R_API bool r_esil_setup(REsil *esil, RAnal *anal, int romem, int stats, int nonu | |||
| esil->cb.reg_write = internal_esil_reg_write_no_null; | |||
| esil->cb.mem_read = internal_esil_mem_read_no_null; | |||
| esil->cb.mem_write = internal_esil_mem_write_no_null; | |||
| r_esil_set_reg_write_imp (esil, (REsilImpHookRegWriteCB)internal_esil_reg_write_no_null, esil); | |||
There was a problem hiding this comment.
ideally the cast shuoldnt be needed, otherwise we can pass functions with different signature and get runtime crashes hard to debug
| R_FREE (esil->hooks->mem_read_implementation); | ||
| } | ||
|
|
||
| R_API bool r_esil_set_mem_write_imp(REsil *esil, REsilImpHookMemWriteCB imp, void *user) { |
There was a problem hiding this comment.
what about having only a set method and tell which event as argument, index those type of events in an fixed size array like esil->hooks[R_ESIL_EVENT_RR]..
| R_API bool r_esil_set_mem_write_imp(REsil *esil, REsilImpHookMemWriteCB imp, void *user) { | ||
| r_return_val_if_fail (esil && esil->hooks, false); | ||
| if (!esil->hooks->mem_write_implementation) { | ||
| esil->hooks->mem_write_implementation = R_NEW (REsilHook); |
There was a problem hiding this comment.
uhm, only 1 handler per event? we cant have multiple listeners? that seems like not what we want right?
| return true; | ||
| } | ||
|
|
||
| R_API void r_esil_del_reg_write_imp(REsil *esil) { |
There was a problem hiding this comment.
i would prefer to have a smaller API and less functions, we want to do the ADD, DEL, CLEAR operations on RR,RW,MM,MW array of callbacks. this array of pointers can be an RVec, so they are executed in the right order and hold a void*user pointer. im thinking about something like this:
esil.hook(ADD, MR, cb, user);
ADD = enum for the supported operations: ADD, DEL, CLEAR, ..
MR = memory read, just another enum with other stuff
cb = the user provided callback for the given esil operation hook
user = user data pointer associated with the given callback
The cb is used as a uniqueid for each event, this is, you can esil.hook(DEL, cb, NULL);
At the end this is very similar to what REvent, what are the limitations you can think about it?
|
ping, rebase for conflicts, and respond my comments pls |
|
I'll look at this again in the next breaking |
|
can this PR be closed now? |
Maybe this time
Still a bit WIP
Description