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

crypto: remove unused OAES code #9505

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jeffro256
Copy link
Contributor

@SyntheticBird45 reported that the OpenBSD compiler gives warnings about string functions used in oaes_sprintf, as well as use of rand() in oaes_key_gen. Neither of these functions are used in the codebase. Removing the unused OAES functions silences these warnings.

openbsd_oaes_warnings

Copy link
Contributor

@tobtoht tobtoht left a comment

Choose a reason for hiding this comment

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

Was about to PR the same thing.

Copy link
Collaborator

@0xFFFC0000 0xFFFC0000 left a comment

Choose a reason for hiding this comment

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

LGTM

@SyntheticBird45
Copy link
Contributor

stillwarning

Still 1 warning left (if we forget easylogging++).
Build profile:

$ cmake -DBUILD_TESTS=off -DCMAKE_BUILD_TYPE=release-static
$ gmake

@SyntheticBird45 reported that the OpenBSD compiler gives warnings about string functions used in `oaes_sprintf`,
as well as use of `rand()` in `oaes_key_gen`. Neither of these functions are used in the codebase. Removing the
unused OAES functions silences these warnings.
jeffro256 added a commit to jeffro256/monero that referenced this pull request Oct 7, 2024
Calling `oaes_alloc`, then `oaes_key_import_data`, then `oaes_free` calls the functions `calloc` and `free` four times each. This PR rewrites the key expansion function to expand directly into a 240 byte buffer with no allocations. This should speed up Cryptonight on platforms such as Raspberry PI. Hash tests are expanded to test slow hash using software AES on all platforms.

Depends on monero-project#9505
@jeffro256
Copy link
Contributor Author

Oops, I pushed an extra commit. Reverted. @SyntheticBird45 can you test now?

Copy link
Contributor

@SyntheticBird45 SyntheticBird45 left a comment

Choose a reason for hiding this comment

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

Error disappeared. Good job.

Copy link
Collaborator

@0xFFFC0000 0xFFFC0000 left a comment

Choose a reason for hiding this comment

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

LGTM

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.

6 participants