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

fuzz: Add missing include (test/util/setup_common.h) #21358

Merged
merged 1 commit into from
Mar 4, 2021

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Mar 4, 2021

src/test/fuzz/socks5.cpp is using the symbol BasicTestingSetup, which is defined in src/test/util/setup_common.h.

Currently compilation happens to succeed because the needed dependency is indirectly included. Compilation will break as soon as the indirect dependency is broken. According to the dev notes, everything that is used must be included.

Fix the issue by including the missing include.

// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
#include <test/fuzz/util.h>
#include <test/util/setup_common.h>
Copy link
Member

@hebasto hebasto Mar 4, 2021

Choose a reason for hiding this comment

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

Which symbol does require this include?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably none in this file, but adding it in this commit will avoid a rebase in #21003 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Does #21003 touch src/test/fuzz/torcontrol.cpp ?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, but the MakeNoLogFileContext symbol is moved to a different file (via the indirect dependencies)

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK fa59ad5

@fanquake fanquake merged commit 7450a01 into bitcoin:master Mar 4, 2021
@maflcko maflcko deleted the 2103-fuzzInc branch March 4, 2021 15:32
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 4, 2021
…n.h)

fa59ad5 fuzz: Add missing include (test/util/setup_common.h) (MarcoFalke)

Pull request description:

  `src/test/fuzz/socks5.cpp` is using the symbol `BasicTestingSetup`, which is defined in `src/test/util/setup_common.h`.

  Currently compilation happens to succeed because the needed dependency is indirectly included. Compilation will break as soon as the indirect dependency is broken. According to the dev notes, everything that is used must be included.

  Fix the issue by including the missing include.

ACKs for top commit:
  fanquake:
    ACK fa59ad5

Tree-SHA512: 9359d5d288ebc5a53d753ebed1ee8d49ddcfe12aeb56054ea43654c0d915337bb0dce7c8a7178e94711ff8dacd1b3ea0a2871b21b1709cd9786efc0c1ef532b3
@practicalswift
Copy link
Contributor

Post-merge ACK fa59ad5

Thanks for fixing this!

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 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