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

Converting pf commands to rzshell #3467

Merged
merged 1 commit into from
Feb 16, 2024
Merged

Converting pf commands to rzshell #3467

merged 1 commit into from
Feb 16, 2024

Conversation

XVilka
Copy link
Member

@XVilka XVilka commented Apr 18, 2023

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

  • pf. now requires space after dot
  • Any pf command now recognizes named format - it should start from dot, e.g. pfs .bla where bla is the format name
  • Defining new format now requires using pfn <format name> <format>
  • Listing named formats - pfn (before - pf.)
  • Print definition of the named format - pfn <format name>, previously pf.bla
  • Writing data requires using pfw command instead of just pf or pf.
  • pfw now supports both variants: pfw bla.foo 42 and pfw bla.foo=42
  • Removed unused format descriptions for NTFS, FAT, TRX

Test plan

CI is green

Closing issues

Partially addresses #1590
Partially addresses #3842

@XVilka XVilka mentioned this pull request Apr 18, 2023
32 tasks
@XVilka XVilka force-pushed the asan-pf-rzshell branch 3 times, most recently from 45733eb to efb04eb Compare May 9, 2023 23:12
@XVilka XVilka added this to the 0.6.0 milestone May 24, 2023
@XVilka XVilka modified the milestones: 0.6.0, 0.7.0 Jul 24, 2023
@XVilka XVilka added pf Print format (`pf` command) shell labels Aug 1, 2023
@XVilka XVilka force-pushed the asan-pf-rzshell branch 4 times, most recently from 15ab3d1 to 0d59cbf Compare December 6, 2023 15:36
@github-actions github-actions bot added the RzBin label Dec 6, 2023
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.

Nice!

subprojects/rizin-shell-parser/corpus/pf_commands.txt Outdated Show resolved Hide resolved
librz/bin/d/ntfs Show resolved Hide resolved
librz/core/cmd/cmd_type.c Outdated Show resolved Hide resolved
Comment on lines +601 to +605
- text: "pf '3xi foo bar'"
comment: "3-array of structures, each with named fields: 'foo' as hex, 'bar' as int"
- text: "pf 'B (BitFldType)arg_name'"
comment: "Resolve bitfield enum type for the arg_name"
- text: "pf 'E (EnumType)arg_name'"
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we have each named argument as a separate argument of the pf command?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe in theory. But that would require rewriting almost all of the librz/type/format.c, which is an immense task in itself and is a story for another time. Moreover, it would make sense to rethink the format syntax/semantics before that task as well: #783

@XVilka XVilka force-pushed the asan-pf-rzshell branch 2 times, most recently from faf5128 to 377b8c1 Compare January 31, 2024 13:38
@XVilka XVilka force-pushed the asan-pf-rzshell branch 8 times, most recently from 70684fb to 9e41073 Compare February 15, 2024 13:43
@XVilka XVilka changed the title WIP - Converting pf commands to rzshell Converting pf commands to rzshell Feb 15, 2024
@XVilka XVilka force-pushed the asan-pf-rzshell branch 4 times, most recently from 19a288e to fc4f71e Compare February 15, 2024 15:27
@XVilka XVilka marked this pull request as ready for review February 15, 2024 15:32
@XVilka XVilka requested a review from Rot127 February 15, 2024 15:33
Copy link
Member

@wargio wargio left a comment

Choose a reason for hiding this comment

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

nice cleanup

@XVilka XVilka merged commit ceefb5e into dev Feb 16, 2024
48 checks passed
@XVilka XVilka deleted the asan-pf-rzshell branch February 16, 2024 02:31
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.

3 participants