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

px to rzshell #2853

Merged
merged 41 commits into from
Aug 4, 2022
Merged

px to rzshell #2853

merged 41 commits into from
Aug 4, 2022

Conversation

imbillow
Copy link
Contributor

@imbillow imbillow commented Jul 29, 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

3/n of #2813

  • pxd -> pxdw

  • pxd1 -> pxd

  • pxd2 -> pxdh

  • pxd4 -> pxdw

  • pxd8 -> pxdq

  • pxAv -> pxAl

  • pxt. -> removed ?

  • px/ -> keep

  • pxf -> TODO: not found old code ?

Test plan

...

Closing issues

...

imbillow and others added 30 commits July 25, 2022 15:13
- `pxd` -> `pxd 4`
- `pxd<N> [len]` -> `pxd <N> [len]`
- `rz_core_print_cmp`
- `rz_core_print_dump`
- `rz_core_print_hexdump_`
- `rz_core_print_hexdump_byline`
- test for `pxW` `pxH`
- `void rz_print_section` -> `const char *rz_print_section`
Co-authored-by: Riccardo Schirone <[email protected]>
Co-authored-by: Riccardo Schirone <[email protected]>
# Conflicts:
#	librz/core/cmd/cmd_print.c
#	librz/core/cprint.c
#	librz/include/rz_util/rz_print.h
#	librz/main/rz-find.c
#	librz/util/print.c
- `pxAv` -> `pxAl`
- `pxt.` -> removed ?
- `px/` -> keep
- `pxf` -> TODO: not found old code ?
librz/core/cmd/cmd_print.c Outdated Show resolved Hide resolved
librz/core/cmd/cmd_print.c Outdated Show resolved Hide resolved
librz/core/cmd/cmd_print.c Show resolved Hide resolved
librz/core/cmd/cmd_print.c Show resolved Hide resolved
Copy link
Member

@ret2libc ret2libc left a comment

Choose a reason for hiding this comment

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

Will review

librz/core/cprint.c Outdated Show resolved Hide resolved
Comment on lines +1072 to +1076
RZ_API RZ_OWN char *rz_core_print_hexdump_diff_str(RZ_NONNULL RzCore *core, ut64 aa, ut64 ba, ut64 len);
RZ_API RZ_OWN char *rz_core_print_dump_str(RZ_NONNULL RzCore *core, RzOutputMode mode, ut64 addr, ut8 n, int len, RzCorePrintFormatType format);
RZ_API RZ_OWN char *rz_core_print_hexdump_or_hexdiff_str(RZ_NONNULL RzCore *core, RzOutputMode mode, ut64 addr, int len, bool use_comment);
RZ_API RZ_OWN char *rz_core_print_hexdump_byline_str(RZ_NONNULL RzCore *core, bool hex_offset, ut64 addr, int len, ut8 size);

Copy link
Member

Choose a reason for hiding this comment

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

IIRC we said these functions should actually print and not be called _str (probably _print). Cutter or others can use internal functions like rz_print_hexdiff_str and similar, right?

Copy link
Member

Choose a reason for hiding this comment

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

@ret2libc if they print they should not be an API, if they should be an API they should return a string. Having an API returning nothing and just printing to the screen is absolutely useless from the user point of view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved these to librz/core/core_private.h

Copy link
Member

Choose a reason for hiding this comment

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

@XVilka we use those kind of rzcore APIs in cases where we need to print the same things in different places (e.g. rz_core_asm_plugins_print). But yes, if we think it's not needed right now, let's not make them APIs.

@XVilka XVilka merged commit 0908b65 into dev Aug 4, 2022
@XVilka XVilka deleted the rzshell_px branch August 4, 2022 01:11
@XVilka XVilka mentioned this pull request Aug 4, 2022
32 tasks
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