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

Remedy some GCC warnings #9

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

Conversation

jeffro256
Copy link

@jeffro256 jeffro256 commented May 28, 2022

The #ifdef SMALLTABLES change eliminates the unused variable warning, and the changes to the sc25519_window4 prototype silences the -Wstringop-overflow= warnings in GCC.

The prototype seems to be wrong b/c it requires an array of size 85, but the function definition only uses 64 bytes, the caller expects it only needs 64 bytes, and the 85 seems to be a copy/paste issue from window3 (Props to @moneromooo-monero for finding that)

Closes #7 and #8

@selsta
Copy link

selsta commented May 28, 2022

Can you add closing #8 to your PR description?

The `#ifdef SMALLTABLES` change eliminates the unused variable warning, and the changes to the sc25519_window4 prototype silences the `-Wstringop-overflow=` warnings in GCC.

The prototype seems to be wrong b/c it requires an array of size 85, but the function definition only uses 64 bytes,  the caller expects it only needs 64 bytes, and the 85 seems to be a copy/paste issue from window3 (Props to @moneromooo-monero for finding that)

Resolves monero-project#8
@selsta
Copy link

selsta commented May 28, 2022

Sorry, #7 too :D And you can just change the PR description, you don't have to force push the patch description.

@jeffro256
Copy link
Author

To explain the ecd change: ecd is used below in that file if and only if SMALLTABLES is defined

@vtnerd
Copy link
Collaborator

vtnerd commented Sep 7, 2022

These files are unmodified from the upstream version. So the only downside is that a diff of the directories will now report these files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants