[WIP] Colorize function arguments and variables part 2 #10777
Conversation
|
If you have any idea on how to improve the code, please don't hesitate to tell it, I'm very unsure of some parts especially inside r_print_colorize_opcode |
cyanpencil
force-pushed
the
colorize-arguments-2
branch
2 times, most recently
from
ca97c64
to
c33d6f4
Compare
libr/util/print.c
Outdated
| @@ -1629,6 +1631,26 @@ static bool issymbol(char c) { | |||
| } | |||
| } | |||
|
|
|||
| static bool check_arg_name (char *p, RList *vars) { | |||
There was a problem hiding this comment.
Can u explain me why do we need this function? (i couldn't understand that's why asking)
There was a problem hiding this comment.
So this function gets called from the r_colorize_opcode() loop when an arg of disasmed instruction is encountered. It returns true if it is equal to the name of one of the local vars.
I've added it to not clutter too much the r_colorize_opcode loop, but it is not necessary at all
| r_list_join (list, reg_vars); | ||
| r_list_join (list, bpv_vars); | ||
| r_list_join (list, spv_vars); | ||
| r_list_free (reg_vars); |
libr/util/print.c
Outdated
| p[z] = '\0'; | ||
| RAnalVar *var; | ||
| RListIter *iter; | ||
| r_list_foreach (vars, iter, var) { |
There was a problem hiding this comment.
I think instead of iterating through list of vars , us this function r_anal_var_get_byname , it's faster since it uses sdb query to retrieve the var , also by doing this way we can avoid this hack and drop RList *vars from RPrint
There was a problem hiding this comment.
Hey this is a great idea, it solves a lot of problems!
Now I'm going to sleep, tomorrow I'll modify it for sure.
Thanks :)
cyanpencil
force-pushed
the
colorize-arguments-2
branch
from
b6a6a46
to
54f91ed
Compare
| RList *reg_vars = r_anal_var_list (anal, fcn, R_ANAL_VAR_KIND_REG); | ||
| RList *bpv_vars = r_anal_var_list (anal, fcn, R_ANAL_VAR_KIND_BPV); | ||
| RList *spv_vars = r_anal_var_list (anal, fcn, R_ANAL_VAR_KIND_SPV); | ||
| r_list_join (list, reg_vars); |
There was a problem hiding this comment.
you can remove one of those lists if you just do RList *list = reg_vars; in the first line
There was a problem hiding this comment.
RList *list is not an empty pointer, it is another list declared some lines before, that needs to be merged to reg_vars, bpv_vars, spv_vars...
|
please fix the comment and rebase. should be green |
Use new colors for comments that hint type of data
fix whitespace
cyanpencil
force-pushed
the
colorize-arguments-2
branch
from
71833cc
to
6e04a55
Compare
|
ok! then ok to merge. but i would like to squash it, it is fine? |
…eorg#10777)" This PARTIALLY reverts commit 13a2cb3. All the updates to cons/d/* files were not reverted.
NOTE: I put it as [WIP] as this needs reviewing, there are some part which are a bit hackish
Changes in this pr:
Comments which indicate the type in disasm are colorized. However to do this, I was forced to add another
R_META_TYPE, different from the normal comment, or ds_show_comments_right wouldn't be able to understand it was a special kind of comment that should be colorizedRenamed new colors from
func_arg*tofunc_var*, which makes more sense.Colorized local vars and args in disasm. This is the hackish bit. To achieve this, I had to pass the list of local vars to
r_print_colorize_opcode(). I added a RList field to RPrint which gets populated with the vars. Then, inside r_print_colorize_opcode(), every argument is confronted with var names through strcmp. This is what needs reviewing the most: I fear this is slow, and not in the spirit of r2, passing variables around like this. But I really couldn't find a better way. Any help is appreciated.Manually modified each theme so that the new colors would fit with the style of each theme. This took some time, but it was worth it, r2 should be as beautiful as possible <3
Example screen of theme
sepia:This works even with custom renamed vars, as you can see from the screen
custom_renamed_var.Please if you have a bit of time cycle all colorschemes with
Rkey to see if some colors mismatch (remember to use scr.color=3 though).