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

Add big numbers in rz-ax #3437

Closed
wants to merge 1 commit into from

Conversation

YoussefMoRabie
Copy link

@YoussefMoRabie YoussefMoRabie commented Mar 28, 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

...

Test plan

...

Closing issues

Closes #3331

librz/main/rz-ax.c Outdated Show resolved Hide resolved
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.

The code is unused and not useful - it prints back the same string. You need to expose also all arithmetics from rz-ax to be able to operate on bitvectors as well.

}
// Get the length of the bitstring
size_t bitlen = strlen(bitstring) - 2;
// Create a bitvector of the appropriate size
Copy link
Member

Choose a reason for hiding this comment

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

Please use clang-format to fix those indentation and space fixes char *name instead of char* name

Copy link
Author

@YoussefMoRabie YoussefMoRabie Mar 28, 2023

Choose a reason for hiding this comment

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

Do you maen a specific arithmetic operations that need to be implemented and how they can be exposed in the rz-ax ?
Because I still don't understand what exactly needs to be implemented

Copy link
Member

Choose a reason for hiding this comment

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

nono, this is regarding the C code format, not the arithmetic ops in rz-ax

Copy link
Author

Choose a reason for hiding this comment

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

nono, this is regarding the C code format, not the arithmetic ops in rz-ax

I already edit my code to use clang-format

librz/main/rz-ax.c Outdated Show resolved Hide resolved
@YoussefMoRabie YoussefMoRabie force-pushed the my-feature branch 2 times, most recently from c151c2e to 44f6edc Compare March 28, 2023 14:45
librz/main/rz-ax.c Outdated Show resolved Hide resolved
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.

Looks great, now integrate its usage with the command line and add a test with a very long sequence of bits.

librz/main/rz-ax.c Outdated Show resolved Hide resolved
librz/main/rz-ax.c Outdated Show resolved Hide resolved
librz/main/rz-ax.c Outdated Show resolved Hide resolved
@wargio
Copy link
Member

wargio commented Mar 31, 2023

add a test under test/db/tools/rz_ax with a very long sequence of bits.

@wargio wargio reopened this Mar 31, 2023
@YoussefMoRabie
Copy link
Author

YoussefMoRabie commented Mar 31, 2023

add a test under test/db/tools/rz_ax

I will do it, and Sorry I closed it by mistake.

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.

The whole concept is wrong. It should be:

  • Remove -L option and use bitvectors for all 0b numbers, and corresponding options
  • It should support arithmetic operations, based on bitvectors and bignum for non-binary numbers
  • It might require some changes in the RzNum API as well

See #3455

@XVilka XVilka closed this Aug 4, 2023
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.

Add big numbers (and/or bitvectors) in rz-ax
3 participants