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

Sanitizer support #2

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Sanitizer support #2

wants to merge 17 commits into from

Conversation

mariopil
Copy link
Collaborator

No description provided.

@@ -0,0 +1,77 @@
#
# Copyright Soramitsu Co., Ltd. All Rights Reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is this coming from? Didn't we have an existing CMakeLists.txt ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Upon reflection, I don't think we should be changing this repo for eqlabs/polkadot-light-client#10 at all - we aren't going to use it anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

cpp-libp2p is using it. It needs update if we want to have sanitizer enabled here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, but cpp-libp2p has more features than we need, and this dependency is for them - IOW I don't think we want to have sanitizer enabled here. @GraDKh?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well it worth enabling sanitizer in all the code that is being compiled. But I just came to a thought that we don't need to add some flags in build system here. We can just pass compile and linker flags via cmake in cpp-libp2p

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, so no update in this repo then, just pass all compiler and linker flags from cpp-libp2p.

Copy link
Collaborator

Choose a reason for hiding this comment

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

fine by me

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW if it works, is there any reason it shouldn't be done for soralog as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants