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.

Sign up for GitHub

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

Jump to bottom

Remove zignatures and move aaF[l] under F[al] and zf[sdc] under F[sdc] #2682

Merged
merged 1 commit into from
Jun 28, 2022
Merged

Remove zignatures and move aaF[l] under F[al] and zf[sdc] under F[sdc] #2682

merged 1 commit into from
Jun 28, 2022

Conversation

wargio
Copy link
Member

@wargio wargio commented Jun 8, 2022 •
edited
Loading

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

command changed:

  • zfc to Fc <outfile> FLIRT create signature file
  • zfd to Fd <infile> FLIRT dump signature file
  • zfs to Fs <infile> FLIRT apply signature file
  • aaF to Fa <filter> SigDB apply signatures
  • aaFl to Fl SigDB list signatures

Fix #272
Fix #973
Fix #1311
Fix #1300

@wargio wargio added this to the 0.5.0 milestone Jun 8, 2022
@ret2libc
Copy link
Member

ret2libc commented Jun 9, 2022

So we want to just kill zignatures completely?

ret2libc
ret2libc reviewed Jun 9, 2022
View reviewed changes
librz/include/rz_flirt.h Outdated Show resolved Hide resolved
@wargio
Copy link
Member Author

wargio commented Jun 9, 2022

So we want to just kill zignatures completely?

yes.

@wargio wargio changed the title Remove zignatures and moved FLIRT core cmds zf under F Remove zignatures and move flirt core cmds zf under F Jun 9, 2022
XVilka
XVilka requested changes Jun 10, 2022
View reviewed changes
librz/signature/sign.c Outdated Show resolved Hide resolved
@wargio wargio requested review from XVilka and ret2libc June 10, 2022 16:21
ret2libc
ret2libc requested changes Jun 13, 2022
View reviewed changes
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.

I'm ok with everything in the PR, apart that if there is a rz_sign library there should probably be a rz_sign.h header.

@wargio
Copy link
Member Author

wargio commented Jun 13, 2022

makes sense, i will revert it back that change.

@XVilka XVilka added the waiting-for-author Used to mark PRs where more work is needed label Jun 15, 2022
@wargio wargio requested a review from ret2libc June 19, 2022 11:31
@XVilka XVilka removed the waiting-for-author Used to mark PRs where more work is needed label Jun 19, 2022
@ret2libc
Copy link
Member

I don't like too much having aaF and aaFl separated from F. I think all the FLIRT management stuff should be in one place. WDYT?

@wargio
Copy link
Member Author

wargio commented Jun 20, 2022 •
edited
Loading

I don't like too much having aaF and aaFl separated from F. I think all the FLIRT management stuff should be in one place. WDYT?

i was actually thinking the same, probably is better to rename everything like:

  • Ffc <outfile> FLIRT create signature file
  • Ffd <infile> FLIRT dump signature file
  • Ffs <infile> FLIRT apply signature file
  • Fds <filter> SigDB apply signatures
  • Fdl SigDB list signatures

@wargio wargio changed the title Remove zignatures and move flirt core cmds zf under F Remove zignatures and move aaF[l] under Fd[sl] and zf[sdc] under Ff[sdc] Jun 22, 2022
@wargio wargio requested review from ret2libc and XVilka June 22, 2022 12:11
ret2libc
ret2libc reviewed Jun 22, 2022
View reviewed changes
librz/core/cmd_descs/cmd_flirt.yaml Outdated Show resolved Hide resolved
librz/core/cmd_descs/cmd_flirt.yaml Outdated Show resolved Hide resolved
librz/core/cmd_descs/cmd_flirt.yaml Outdated Show resolved Hide resolved
librz/core/cmd_descs/cmd_flirt.yaml Outdated Show resolved Hide resolved
librz/core/cmd_descs/cmd_flirt.yaml Outdated Show resolved Hide resolved
@wargio wargio force-pushed the remove-zign branch 2 times, most recently from 7869df8 to 6ab3024 Compare June 25, 2022 11:33
@wargio wargio changed the title Remove zignatures and move aaF[l] under Fd[sl] and zf[sdc] under Ff[sdc] Remove zignatures and move aaF[l] under F[al] and zf[sdc] under F[sdc] Jun 25, 2022
@wargio wargio requested a review from ret2libc June 25, 2022 15:29
ret2libc
ret2libc reviewed Jun 27, 2022
View reviewed changes
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.

Can we rename the folder to librz/sign? Also, do I remember wrong or we agreed to keep the rz_sign.h header for consistency with the name of the rz-module?

@wargio
Copy link
Member Author

wargio commented Jun 27, 2022

Can we rename the folder to librz/sign? Also, do I remember wrong or we agreed to keep the rz_sign.h header for consistency with the name of the rz-module?

Yes for rz_sign and and is in there.

ret2libc
ret2libc requested changes Jun 28, 2022
View reviewed changes
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.

Please rename librz/signature to librz/sign, apart from that this LGTM.

@XVilka XVilka merged commit ea02b0d into dev Jun 28, 2022
@XVilka XVilka deleted the remove-zign branch June 28, 2022 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ret2libc ret2libc ret2libc requested changes

@XVilka XVilka XVilka approved these changes

@thestr4ng3r thestr4ng3r Awaiting requested review from thestr4ng3r thestr4ng3r is a code owner

@kazarmy kazarmy Awaiting requested review from kazarmy kazarmy is a code owner

Assignees

@ret2libc ret2libc

Labels
API FLIRT rizin rz-test RzAnalysis RzCore
Projects
None yet
Milestone
0.5.0
3 participants
@wargio @ret2libc @XVilka