Remove pointer cast in CRPCTable::dumpArgMap #21035
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
No change in behavior
No change in behavior. New function is split from CRPCTable::execute and used in the next commit.
CRPCTable::dumpArgMap currently works by casting RPC command unique_id integer field to a function pointer, and then calling the function. The unique_id field wasn't supposed to be used this way (it's meant to be used to detect RPC aliases), and this code segfaults in the rpc_help.py test in multiprocess PR bitcoin#10102 because wallet RPC functions aren't directly accessible from the node process. Fix this by adding a new GET_ARGS request mode to retrieve argument information similar to the way the GET_HELP mode retrieves help information.
|
Thanks, Concept ACK |
laanwj
reviewed
Feb 1, 2021
laanwj
reviewed
Feb 1, 2021
|
Concept ACK. |
maflcko
approved these changes
Feb 23, 2021
There was a problem hiding this comment.
review ACK 8a6e870 🥅
Show signature and timestamp
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
review ACK 8a6e870cbb34586958ca4c17037d41df800a26ef 🥅
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUirIAv/aoT2DpLO8cDSa87LK8Wr/CEGuuvUnfZ0B5s1r34wjIxMUI+XWmwV+Hhu
UXTQT0P0JY84TXxaYiXYVqH27qiVU6N66AXTi4s6X4W0Ndjl6ZUo9aNYScahyDgX
iMbw4TiGrPzjhNH0kxA5De5Q/Frs90eM5kUGzlahVPwyNg+7RI5kZ7Gqi7yVRouX
WN+pjxRK7RXZTlZDnFQz9vbZQLbMexnnoRA5RZ/7h1mul1PbaEWjCBDybVHncRkA
a0+WKKA9yPDnLpn26o2/kOu2ChQ+7E4FraE1WIM7+iy6p7JnVtMDlk1VtfvZek1d
XL9mV6E6aUrqOnTjfssk8yl6VLmjFpiDCW+fYuaoxfBPjvHRTTOqGzKnV+0s8Voh
IJdfqoIkkCp9GMoQHn2PPWliq99LRGjWlZsYxSer/oUwE6BlZaKqf3cprN1d6DbI
0MmK4Yxpte15H2WuoDG/1IiwFjIQxM8rot7Y1pwOblZMFmi6EeV4tlOYZnnBBqLw
tieUn9Ym
=gOJt
-----END PGP SIGNATURE-----
Timestamp of file with hash a3e1a8004872dac4b7123281ccd1da57e1dcc6b23121c63d3ced4954ec92a7cb -
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
ryanofsky
commented
Mar 9, 2021
There was a problem hiding this comment.
Updated 8a6e870 -> 9048c58 (pr/argmap.1 -> pr/argmap.2, compare) with review suggestions
|
re-ACK 9048c58 👑 Show signature and timestampSignature: Timestamp of file with hash |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Mar 15, 2021
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 5, 2022
Summary: No change in behavior This is a backport of [[bitcoin/bitcoin#21035 | core#21035]] [1/3] bitcoin/bitcoin@6158a6d Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10764
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 5, 2022
Summary: No change in behavior. New function is split from `CRPCTable::execute` and used in the next commit. This is a backport of [[bitcoin/bitcoin#21035 | core#21035]] [2/3] bitcoin/bitcoin@14f3d9b Depends on D10764 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10765
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 5, 2022
Summary: `CRPCTable::dumpArgMap` currently works by casting RPC command `unique_id` integer field to a function pointer, and then calling the function. The `unique_id` field wasn't supposed to be used this way (it's meant to be used to detect RPC aliases), and this code segfaults in the `rpc_help.py` test in multiprocess PR [[bitcoin/bitcoin#10102 | core#10102]] because wallet RPC functions aren't directly accessible from the node process. Fix this by adding a new `GET_ARGS` request mode to retrieve argument information similar to the way the `GET_HELP` mode retrieves help information. This is a backport of [[bitcoin/bitcoin#21035 | core#21035]] [3/3] bitcoin/bitcoin@9048c58 Depends on D10765 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10766
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change is needed to fix the
rpc_help.pytest failing in #10102: https://cirrus-ci.com/task/5469433013469184?command=ci#L2275The
CRPCTable::dumpArgMapmethod currently works by casting RPCunique_idinteger field to a function pointer, and then calling it. Theunique_idfield wasn't supposed to be used this way (it's meant to be used to detect RPC aliases) and as a result, this code segfaults in therpc_help.pytest in multiprocess PR #10102 because wallet RPC functions aren't directly accessible from the node process.Fix this by adding a new
GET_ARGSRPC request mode to retrieve argument information similar to the way theGET_HELPmode retrieves help information.This PR is part of the process separation project.