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

Bug27772 #356

Closed
wants to merge 9 commits into from
Closed

Bug27772 #356

wants to merge 9 commits into from

Conversation

nmathewson
Copy link
Contributor

No description provided.

Instead, have it call a mockable function.  We don't want
crypto_strongest_rand() to be mockable, since doing so creates a
type error when we call it from ed25519-donna, which we do not build
in a test mode.

Fixes bug 27728; bugfix on 0.3.5.1-alpha
GCC got confused here with LTO enabled.

Fixes part of #27772.
The trunnel functions are written under the assumption that their
allocators can fail, so GCC LTO thinks they might return NULL.  In
point of fact, they're using tor_malloc() and friends, which can't
fail, but GCC won't necessarily figure that out.

Fixes part of #27772.
This is now officially an antipattern: please let's never copy a
function declaration in two places again.  That's what headers are
for.
These confused GCC LTO, which thought they might be used
uninitialized.  I'm pretty sure that as long as 'res' indicates
success, they will always be set to something, but let's unconfuse
the compiler in any case.
Nothing should ever look at them on failure, but in some cases,
the unit tests don't check for failure, and then GCC-LTO freaks out.

Fixes part of 27772.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 61.894% when pulling 0f06ec8 on nmathewson:bug27772 into 50367d0 on torproject:master.

@nmathewson
Copy link
Contributor Author

Squashed and merged

@nmathewson nmathewson closed this Oct 14, 2018
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.

2 participants