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

Fix COFF reloc naming #4701

Merged
merged 2 commits into from
Nov 5, 2024
Merged

Fix COFF reloc naming #4701

merged 2 commits into from
Nov 5, 2024

Conversation

Roeegg2
Copy link
Contributor

@Roeegg2 Roeegg2 commented Nov 5, 2024

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

Not very big changes, explanations in commit descriptions

...

Test plan

All tests green
...

Closing issues

N/A
...

@Roeegg2 Roeegg2 changed the title Coff reloc naming COFF reloc naming Nov 5, 2024
@Roeegg2 Roeegg2 changed the title COFF reloc naming Fix COFF reloc naming Nov 5, 2024
@Roeegg2
Copy link
Contributor Author

Roeegg2 commented Nov 5, 2024

@wargio fixed everything, let me know what you think

wargio
wargio requested changes Nov 5, 2024
View reviewed changes
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.

just add the default case in all the switches and can be merged.

@Roeegg2
Update COFF relocations
7d5eac5 
* Break down `relocs_foreach` to sub functions for readability
* Migrate to using `RzBinReloc.print_name` for reloc name printing
@Roeegg2
Update COFF relocation tests
5ea3ba5
@Roeegg2
Copy link
Contributor Author

Roeegg2 commented Nov 5, 2024

fixed.
I also added a warnings if we reach default

@XVilka XVilka requested a review from wargio November 5, 2024 14:02
@XVilka XVilka merged commit 64a4bb4 into rizinorg:dev Nov 5, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XVilka XVilka XVilka approved these changes

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

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

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

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

Assignees
No one assigned
Labels
rz-test RzBin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Roeegg2 @XVilka @wargio