Skip to content

Conversation

@sriramdvt
Copy link
Contributor

There are 78 cpp files that include util.h (grep -iIr "#include <test/fuzz/util.h>" src/test/fuzz | wc -l). Modifying the implementation of a fuzz helper in src/test/fuzz/util.h will cause all fuzz tests to be recompiled. Keeping the declarations of these non-template fuzz helpers in util.h and moving their implementations to util.cpp will skip the redundant recompilation of all the fuzz tests, and builds these helpers only once in util.cpp.

Functions moved from util.h to util.cpp:

  • ConsumeTxMemPoolEntry
  • ContainsSpentInput
  • ConsumeNetAddr
  • Methods of FuzzedFileProvider::(open, read, write, seek, close)

@fanquake fanquake added the Tests label Jul 14, 2021
Copy link
Contributor

@kristapsk kristapsk left a comment

Choose a reason for hiding this comment

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

utACK 6abf29492fba1408c874972e7c0e2afc4d84501e

@maflcko
Copy link
Member

maflcko commented Jul 14, 2021

Nice. Concept ACK.

Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

Moved implementations of `ConsumeTxMemPoolEntry`, `ContainsSpentInput`, `ConsumeNetAddr`, and the methods(open, read, write, seek, close) of FuzzedFileProvider from test/fuzz/util.h to test/fuzz/util.cpp.
@theStack
Copy link
Contributor

Concept ACK

@maflcko
Copy link
Member

maflcko commented Jul 16, 2021

review ACK a2aca20 🍂

reviewed with:
git show a2aca20 --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

review ACK a2aca207b1ad00ec05d7533dbd75bbff830e1d75 🍂

reviewed with:
git show a2aca207b1ad00ec05d7533dbd75bbff830e1d75 --color-moved=dimmed-zebra  --color-moved-ws=ignore-all-space
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUiU8wv/exBlSHJsxQWHuaqEY1I9i6MgEuRCL0kwKrGkqcFksqEi+m+9twfOKDwX
PSg9yhSAGkBnEMefZjfuvY49UpghZ5R/8ur4+bm3nSUkwVeq55MmUL6GO6cGzKzx
vU24RhvVwB1eMZTX6o6KDOumKpYtRXTm09+t+L/NsWbnt+o/azWPKsV2UUU/wsC0
0Ph1i5SgFgpMd3pfXzGrU6/AOmK5CqN0dK5NGj/pEfF8cAm5N+BCRVI49FvCAu8c
5rKdlcnpHW6tcDCPxcKH2im/CVQx6SXZBRsh2I8KH/OVgGFRGLpxvbVUazIuVVKE
LsezETOEqio6wZacpBXSSVV/eh1kNhs45NVj2vSmYq2Mbx2l6XdMLIEt4mk42IEZ
L7Daxky18E5xnCG1GChWf0CBa70PwUScSrJAJaEyt7cP08WvSYOR7f5WmqJ4eBjK
pMKwwVEuSVXZFNF67X7Yeav4lMkZjA8JtQFFuXj0XuTgksr48JgkTZCfDzI4f8t/
OeaXoZEC
=OjnR
-----END PGP SIGNATURE-----

Timestamp of file with hash a68bd1199081ef367925d3f633cddc5d34c485b33bbba693b151eb7f0eb39a95 -

@maflcko maflcko merged commit 0eea1df into bitcoin:master Jul 18, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jul 23, 2021
…helpers from util.h to util.cpp

a2aca20 Move implementations of non-template fuzz helpers (Sriram)

Pull request description:

  There are 78 cpp files that include `util.h` (`grep -iIr "#include <test/fuzz/util.h>" src/test/fuzz | wc -l`). Modifying the implementation of a fuzz helper in `src/test/fuzz/util.h` will cause all fuzz tests to be recompiled. Keeping the declarations of these non-template fuzz helpers in `util.h` and moving their implementations to `util.cpp` will skip the redundant recompilation of all the fuzz tests, and builds these helpers only once in `util.cpp`.

  Functions moved from `util.h` to `util.cpp`:
  - `ConsumeTxMemPoolEntry`
  - `ContainsSpentInput`
  - `ConsumeNetAddr`
  - Methods of `FuzzedFileProvider::(open, read, write, seek, close)`

ACKs for top commit:
  MarcoFalke:
    review ACK a2aca20 🍂

Tree-SHA512: e7037ebb86d0fc56048e4f3d8733eefc21da11683b09d2b22926bda410719628d89c52ddd9b4c18aa243607a66fdb4d13a63e62ca010e66b3ec9174fd18107f0
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
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.

5 participants