Skip to content

Conversation

Fixes bitcoin#27472

Signed-off-by: Pttn <28868425+Pttn@users.noreply.github.com>

Github-Pull: bitcoin#27473
Rebased-From: 0d6383f
@fanquake fanquake added this to the 24.1 milestone Apr 17, 2023
@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 17, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK stickies-v, hebasto, jonatack

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@jonatack
Copy link
Member

Suggest adding f73782a.

Compiling bdb with clang-16 on aarch64 (hardware) currently fails:
```bash
make -C depends/ bdb CC=clang CXX=clang++
...
checking for mutexes... UNIX/fcntl
configure: WARNING: NO SHARED LATCH IMPLEMENTATION FOUND FOR THIS PLATFORM.
configure: error: Unable to find a mutex implementation
```

Looking at config.log we've got:
```bash
configure:18704: checking for mutexes
configure:18815: clang -o conftest -pipe -std=c11 -O2 -Wno-error=implicit-function-declaration -Wno-error=format-security    -I/bitcoin/depends/aarch64-unknown-linux-gnu/include -D_GNU_SOURCE -D_REENTRANT   -L/bitcoin/depends/aarch64-unknown-linux-gnu/lib conftest.c  -lpthread >&5
conftest.c:45:1: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
main() {
^
int
conftest.c:50:2: warning: call to undeclared library function 'exit' with type 'void (int) __attribute__((noreturn))'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
        exit (
        ^
conftest.c:50:2: note: include the header <stdlib.h> or explicitly provide a declaration for 'exit'
1 warning and 1 error generated.
```

Clang-16 changed `-Wimplicit-function-declaration` and `-Wimplicit-int`
warnings into errors, see:
https://releases.llvm.org/16.0.0/tools/clang/docs/ReleaseNotes.html#potentially-breaking-changes.

> The -Wimplicit-function-declaration and -Wimplicit-int warnings now
> default to an error in C99, C11, and C17. As of C2x, support for implicit
> function declarations and implicit int has been removed, and the
> warning options will have no effect. Specifying -Wimplicit-int in
> C89 mode will now issue warnings instead of being a noop.

Github-Pull: bitcoin#27462
Rebased-From: f8b8458
@fanquake fanquake force-pushed the 24_1_rc2_backports branch from 61dded6 to 5c937ae Compare April 18, 2023 09:09
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 5c937ae

@stickies-v
Copy link
Contributor

stickies-v commented Apr 18, 2023

Suggest adding stickies-v@db6cf79 which backports #27468, but without the changes to rest_mempool (query parameters in this endpoint were added only after v24 in #26207).

Edit: this backport is relevant only for 24.x, not for 23.x.

pablomartin4btc and others added 3 commits April 18, 2023 11:43
`evhttp_uri_parse` can return a nullptr, for example when the URI
contains invalid characters (e.g. "%").
`GetQueryParameterFromUri` passes the output of `evhttp_uri_parse`
straight into `evhttp_uri_get_query`, which means that anyone calling
a REST endpoint in which query parameters are used (e.g. `rest_headers`)
can cause a segfault.

This bugfix is designed to be minimal and without additional behaviour change.

Github-Pull: bitcoin#27468
Rebased-From: 11422cc
- clarify that there can be multiple warning messages
- specify the correct wallet action
- describe the use of newlines as delimiters

Github-Pull: bitcoin#27279
Rebased-From: f73782a
@fanquake
Copy link
Member Author

Suggest adding stickies-v@db6cf79 which backports #27468,

Pulled & added metadata.

Suggest adding f73782a.

Ok.

@fanquake fanquake force-pushed the 24_1_rc2_backports branch from 5c937ae to dc711fb Compare April 18, 2023 11:30
Copy link
Contributor

@stickies-v stickies-v left a comment

Choose a reason for hiding this comment

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

ACK dc711fb

Verified that the backports correspond to their original PR, and I can't see any silent merge conflicts. Release notes match the backported changes.

@DrahtBot DrahtBot requested a review from hebasto April 18, 2023 13:43
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

re-ACK dc711fb

Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

ACK dc711fb

Pull description is missing some of the changes.

@fanquake
Copy link
Member Author

This has been merged.

@fanquake fanquake closed this Apr 18, 2023
@fanquake fanquake deleted the 24_1_rc2_backports branch April 18, 2023 14:33
fanquake added a commit to fanquake/bitcoin that referenced this pull request Apr 18, 2023
dc711fb doc: update 24.1 release notes (fanquake)
fc8c1a8 doc: fix/improve warning helps in {create,load,unload,restore}wallet (Jon Atack)
3a26b19 bugfix: rest: avoid segfault for invalid URI (pablomartin4btc)
c40b1da depends: fix compiling bdb with clang-16 on aarch64 (fanquake)
0bac52d Don't return OutputType::UNKNOWN in ParseOutputType (Pttn)

Pull request description:

  Backports:
  * bitcoin#27279 (only f73782a)
  * bitcoin#27462
  * bitcoin#27468
  * bitcoin#27473

ACKs for top commit:
  stickies-v:
    ACK dc711fb
  hebasto:
    re-ACK dc711fb
  jonatack:
    ACK dc711fb

Tree-SHA512: 72c673be82689e3c3a1c2564a1fdd6afe0b357b7aa8bec9524fe6999804fbccf310da0b074e647af14b753e5e695024e268fe4f69aa58747f541f7f429ebede6
fanquake added a commit that referenced this pull request Apr 19, 2023
03a16a1 doc: update release notes for 24.1rc2 (fanquake)
f344c66 doc: update manual pages for v24.1rc2 (fanquake)
4451e89 build: bump version to v24.1rc2 (fanquake)

Pull request description:

  rc1 (tagged > a month ago) was mostly a no-op, due to lack of binaries. While some are up now, [albeit at the wrong URL](https://bitcoincore.org/bin/bitcoin-24.1rc1/), there seems no point continuing the rc1 cycle, when we already have additional changes backported (#27474), and we've also received no bug reports/feedback from anyone who did test.

  So bump the version, and cut an rc2.

ACKs for top commit:
  achow101:
    ACK 03a16a1
  gruve-p:
    ACK 03a16a1
  hebasto:
    ACK 03a16a1, I have reviewed the code and it looks OK.

Tree-SHA512: f49072149ecabb02b034b1c4d7319a80f93a8c7a29a2b7ec27dff0c257742d08d48fbf266399ce71769cec992902b7d53fc26bb5ffc8681728dc8685cbba25d9
@bitcoin bitcoin locked and limited conversation to collaborators Apr 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants