"Schnorrsig API overhaul" fixups #963

pull real-or-random wants to merge 5 commits into bitcoin-core:master from real-or-random:202106-initializer-lists changing 7 files +27 −6

real-or-random commented at 12:14 AM on July 4, 2021: contributor

Fixups for #844 .

Fixes #962 .

Now I wonder if a static const in the header file would be better than a macro.

By the way, an unrelated issue is that we should have extern C as we do in the main header...
Fix -Wmissing-braces warning in clang 2cc3cfa583
gmaxwell commented at 1:39 AM on July 4, 2021: contributor

LGTM. Maybe you should add a CI build that builds the entire project/tests/benchmarks with a C++ compiler-- it should compile. It may need -fpermissive or a few extra explicit casts. (Don't use Werror, though).

Don't use string literals for char arrays without NUL termination

unsigned char foo[4] = "abcd" is not valid C++ because the string
literal "abcd" does not fit into foo due to the terminating NUL
character. This is valid in C, it will just omit the NUL character.

Fixes #962.

769528f307

Don't declare constants twice
This is forbidden in C++.
b5b8e7b719

Use unsigned char consistently for byte arrays

C++ does not allow initialization with string literals but we do it in other
places and -fpermissive will convince g++ to compile.

f698caaff6

elichai commented at 10:14 AM on July 4, 2021: contributor

utACK 792d7c46668144e0ca2a321b0765935037258d2f (currently the CI fails on master and this fixes it)
real-or-random commented at 12:27 PM on July 4, 2021: contributor

Not ready to merge, I'll have some uncommitted additional changes but had to interrupt my work.
real-or-random force-pushed on Jul 5, 2021
real-or-random force-pushed on Jul 5, 2021
real-or-random force-pushed on Jul 5, 2021
ci: Add C++ test 90e83449b2
real-or-random force-pushed on Jul 5, 2021
jonasnick commented at 8:30 PM on July 5, 2021: contributor

Nice, CI output looks as expected.

Fwiw, the issue with CI on master after merging #844 wouldn't have been easily caught before pushing because the -Wmissing-braces warning doesn't appear with the version of clang installed on my system at least (I have version 12, appears with v7).

By the way, an unrelated issue is that we should have extern C as we do in the main header...

We do already.

ACK 90e83449b2c2e4046af755e35fdeee579a468f31
jonasnick merged this on Jul 5, 2021
jonasnick closed this on Jul 5, 2021
sipa referenced this in commit c020cbaa5c on Jul 14, 2021
apoelstra cross-referenced this on Jul 27, 2021 from issue Upstream PRs 879, 959, 955, 944, 951, 960, 844, 963, 965 by apoelstra
apoelstra referenced this in commit 196c993d1f on Jul 28, 2021
janus referenced this in commit 62075fc9aa on Nov 5, 2021
patricklodder referenced this in commit 21badcf9d2 on Jul 25, 2022
patricklodder referenced this in commit 03002a9013 on Jul 28, 2022
str4d referenced this in commit 7648f6bf55 on Apr 21, 2023
vmta referenced this in commit e1120c94a1 on Jun 4, 2023
vmta referenced this in commit 8f03457eed on Jul 1, 2023

Contributors

Linked (view graph)

#844 schnorrsig API overhaul #962 SECP256K1_SCHNORRSIG_EXTRAPARAMS_INIT does not work with C++