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

Convert ag to rzshell #2916

Merged
merged 56 commits into from
Aug 22, 2022
Merged

Convert ag to rzshell #2916

merged 56 commits into from
Aug 22, 2022

Conversation

imbillow
Copy link
Contributor

@imbillow imbillow commented Aug 13, 2022

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository
  • I made sure to follow the project's coding style
  • I've documented or updated the documentation of every function and struct this PR changes. If not so I've explained why.
  • I've added tests that prove my fix is effective or that my feature works (if possible)
  • I've updated the rizin book with the relevant information (if needed)

Detailed description

Before many ag commands had a "format" as a part of the command itself, e.g. agfk - here command was agf while k is the format. For the time being it will become agf k, unless you have better ideas.

ag<graphtype><format> -> ag<graphtype> <format>

but ag<graphtype>w -> agw <graphtype>

Usage: ag<?>   # Analysis graph commands
| aga <format>=           # Data reference graph
| agA <format>=           # Global data references graph
| agc <format>=           # Function callgraph
| agC <format>=           # Global callgraph
| agd [<format> [<addr>]] # Diff graph
| agf <format>=           # Basic blocks function graph
| agi <format>=           # Imports graph
| agr <format>=           # References graph
| agR <format>=           # Global references graph
| ags                     # Unknown graph
| agl                     # Line graph ?
| agx <format>=           # Cross-references graph
| agg <format>=           # Custom graph
| ag-                     # Clear the custom graph
| agn[-]                  # Managing custom graph nodes
| age[-]                  # Managing custom graph edges
| agw <graphtype> <path>  # Write to path or display graph image (see graph.gv.format)

Maybe open another PR to do more refactoring and export some APIs
But this PR mainly focuses on convert to rzshell

Test plan

CI is green

Closing issues

Partially addresses #1442

previous PR #2757

librz/core/cmd_descs/cmd_analysis.yaml Outdated Show resolved Hide resolved
@XVilka XVilka added this to the 0.5.0 milestone Aug 14, 2022
@imbillow imbillow marked this pull request as ready for review August 14, 2022 21:48
Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming it's only the first step and there will be some more additional cleanup and refactoring, this PR is OK except for one thing, in my opinion, - the default_value: ' ' of the format. I hope there could be a better solution. cc @ret2libc @wargio

librz/core/cmd/cmd_analysis.c Outdated Show resolved Hide resolved
librz/core/cmd/cmd_analysis.c Outdated Show resolved Hide resolved
@XVilka
Copy link
Member

XVilka commented Aug 18, 2022

@imbillow please rebase on top of the latest dev

@XVilka
Copy link
Member

XVilka commented Aug 19, 2022

@imbillow, why did you remove some tests?

@imbillow
Copy link
Contributor Author

@imbillow, why did you remove some tests?

Because the processing of rz_core_analysis_graph is very problematic, but I don't want to solve it in this PR, so temporarily delete these tests.

@XVilka
Copy link
Member

XVilka commented Aug 19, 2022

@imbillow then please mark them "BROKEN=1" instead of removing and open an issue so they aren't forgotten.

@imbillow
Copy link
Contributor Author

#2938

OK

@XVilka XVilka requested review from ret2libc and wargio August 20, 2022 15:51
librz/diff/distance.c Outdated Show resolved Hide resolved
librz/core/cmd_descs/cmd_descs.h Show resolved Hide resolved
@imbillow imbillow requested a review from ret2libc August 20, 2022 17:54
@imbillow imbillow mentioned this pull request Aug 21, 2022
5 tasks
@XVilka
Copy link
Member

XVilka commented Aug 21, 2022

@ret2libc please take a look again and let's merge this if you don't have any big objections. Further graph improvements can be done in separate PRs.

@XVilka XVilka merged commit 4b38597 into dev Aug 22, 2022
@XVilka XVilka deleted the asan-fuzz-ag-rzshell branch August 22, 2022 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants