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

secp256k1.h: clarify that by default arguments must be != NULL #926

Merged
merged 1 commit into from
May 7, 2021

Conversation

jonasnick
Copy link
Contributor

The same file says that the illegal callback will only triger for violations
explicitly mentioned, which is not true without this commit because we often
don't mention that an argument is not allowed to be NULL.

This line is extracted from #783 in the hope that it gets merged faster because other PRs depend on it.

@gmaxwell
Copy link
Contributor

This seems fine to me, though FWIW, calling any the non-nullable inputs with a statically null input will cause warnings in the caller. And the non-nullable arguments should all be documented as non-nullable with the annotations in the header (though not in doxygen, indeed).

The same file says that the illegal callback will only triger for violations
explicitly mentioned, which is not true without this commit because we often
don't mention that an argument is not allowed to be NULL.
@jonasnick
Copy link
Contributor Author

Agreed. Pushed an unrelated (slight) tweak.

Copy link
Contributor

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

ACK 0881633

@gmaxwell
Copy link
Contributor

gmaxwell commented May 7, 2021

ACK 0881633

@real-or-random real-or-random merged commit 6939487 into bitcoin-core:master May 7, 2021
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 22, 2024
…_cmp`

Summary:
Allows comparing `secp256k1_pubkey` and `secp256k1_xonly_pubkey` respectively, to establish an order between them.

This is a backport of [[ bitcoin-core/secp256k1#850 | secp256k1#850 ]] and [[ bitcoin-core/secp256k1#926 | secp256k1#926 ]].

We need this for porting `rust-secp256k1` to the secp256k1 library of this repo.

Test Plan: `ninja check-secp256k1`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D16957
Fabcien pushed a commit to Bitcoin-ABC/secp256k1 that referenced this pull request Oct 23, 2024
…_cmp`

Summary:
Allows comparing `secp256k1_pubkey` and `secp256k1_xonly_pubkey` respectively, to establish an order between them.

This is a backport of [[ bitcoin-core/secp256k1#850 | secp256k1#850 ]] and [[ bitcoin-core/secp256k1#926 | secp256k1#926 ]].

We need this for porting `rust-secp256k1` to the secp256k1 library of this repo.

Test Plan: `ninja check-secp256k1`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D16957
roqqit pushed a commit to doged-io/doged that referenced this pull request Nov 20, 2024
…_cmp`

Summary:
Allows comparing `secp256k1_pubkey` and `secp256k1_xonly_pubkey` respectively, to establish an order between them.

This is a backport of [[ bitcoin-core/secp256k1#850 | secp256k1#850 ]] and [[ bitcoin-core/secp256k1#926 | secp256k1#926 ]].

We need this for porting `rust-secp256k1` to the secp256k1 library of this repo.

Test Plan: `ninja check-secp256k1`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D16957
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.

3 participants