Skip to content

Trying to prevent accidental usage of the fuzztarget functions #154

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

Closed
wants to merge 2 commits into from

Conversation

elichai
Copy link
Member

@elichai elichai commented Aug 23, 2019

A suggestion of a way to solve #90

This is a result of a conversation with @real-or-random and some feedback from @TheBlueMatt

The part i'm less sure of is the tests here, because rust tests run in concurrent threads and I want to test both with the bool set and without I had to use some Mutex to make sure they don't run together

Another thing that isn't too nice is that we need to put this is all the fuzzing functions.
if proc macros were stabilized before 1.22 we could've wrote an attribute macro and made it a bit nicer but it would've still required doing it on every function.

@elichai elichai force-pushed the 2019-08-fuzz-bool branch from 1f2cb92 to 30193f5 Compare August 23, 2019 21:56
@elichai elichai changed the title 2019 08 fuzz bool Trying to prevent accidental usage of the fuzztarget functions Aug 23, 2019
Copy link
Collaborator

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

The part i'm less sure of is the tests here, because rust tests run in concurrent threads and I want to test both with the bool set and without I had to use some Mutex to make sure they don't run together

Well it's not super elegant but it works, so it's good. This is discussed here: rust-lang/rust#43155 This issue has some nice code snippets. The simplest solution is to pass -- --test-threads=1 to cargo test. I think we could actually do that (with a comment that explains it).

@elichai elichai force-pushed the 2019-08-fuzz-bool branch from 30193f5 to cb3acfd Compare August 24, 2019 14:56
@elichai
Copy link
Member Author

elichai commented Aug 24, 2019

Well it's not super elegant but it works, so it's good. This is discussed here: rust-lang/rust#43155 This issue has some nice code snippets. The simplest solution is to pass -- --test-threads=1 to cargo test. I think we could actually do that (with a comment that explains it).

That's cool :) did not know about this flag.

real-or-random
real-or-random previously approved these changes Aug 24, 2019
@real-or-random real-or-random dismissed their stale review August 24, 2019 15:19

overlooked a nit

@elichai elichai force-pushed the 2019-08-fuzz-bool branch from cb3acfd to cdd89ee Compare August 24, 2019 15:20
real-or-random
real-or-random previously approved these changes Aug 24, 2019
@elichai
Copy link
Member Author

elichai commented Apr 29, 2020

Rebased, and replaced the -- --test-threads=1 trick with just running them both in the same single test.

@elichai elichai force-pushed the 2019-08-fuzz-bool branch from e720016 to 3698599 Compare April 29, 2020 20:55
@elichai elichai force-pushed the 2019-08-fuzz-bool branch from 3698599 to c29481e Compare April 29, 2020 21:03
@elichai elichai force-pushed the 2019-08-fuzz-bool branch from c29481e to d58822e Compare May 13, 2020 22:42
@apoelstra
Copy link
Member

Closing, fixed by #263

@apoelstra apoelstra closed this Jan 11, 2021
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