Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

We found a bug in the hyper HTTP library

blog.cloudflare.com

Post reply on HN
Print view View original

41–50 of 97 posts

Re: We found a bug in the hyper HTTP library

#41

Post by logicchains »

fwlr wrote: post #10
logicchains wrote: post #3

[flagged]

let _ = …? This is the Rust idiom for “I am intentionally ignoring this return value”. The linter would have caught self.poll_read()?; and in fact one of the options the linter itself suggests in this case is exactly this “let underscore equals” idiom. (Arguably, this code exists because of the linter, not due to its absence!) In any case, the return value is being “handled” - the question mark examines the result an…

>This is the Rust idiom for “I am intentionally ignoring this return value”.

That doesn't make the code any less awful, it just makes idiomatic Rust sound awful. Discarding a return value without even a comment to explain why shouldn't be allowed in any critical project, and the linter should be perfectly capable of ensuring that a comment accompanies the discard and complaining loudly when it doesn't.

Re: We found a bug in the hyper HTTP library

#43

Post by Twey »

pwdisswordfishq wrote: post #35
Twey wrote: post #27

This would have been flagged by Clippy lints `let_underscore_untyped` or `let_underscore_must_use`, which sadly are not enabled by default.

Ehh, easy fix #[allow(clippy::let_underscore_untyped,clippy::let_underscore_must_use)] let _ = self.poll_flush(cx)?;

I said ‘flagged’, not ‘fixed’ :)

You can always write the wrong code if you want it enough. But hopefully a warning would have prompted someone to think harder about this flow.

Re: We found a bug in the hyper HTTP library

#45

Post by Twey »

nesarkvechnep wrote: post #38
pwdisswordfishq wrote: post #35

Earlier quoted context omitted.

Ehh, easy fix #[allow(clippy::let_underscore_untyped,clippy::let_underscore_must_use)] let _ = self.poll_flush(cx)?;

Yeah, but you must know about them and the possible bug first in order to allow them...

Hence ‘sadly’. IMNSHO both of these (or at least _untyped) should be enabled by default. Untyped `let _` is too big a footgun during refactorings.

Re: We found a bug in the hyper HTTP library

#46

Post by Twey »

Joker_vD wrote: post #40
nesarkvechnep wrote: post #38

Earlier quoted context omitted.

Yeah, but you must know about them and the possible bug first in order to allow them...

At which point you wouldn't have written this bug in the first place; or the warnings would trigger immediately, you'd change _ to an actual variable and then remove the warning pragmas because now you don't assign to _.

`Poll` is marked `#[must_use]` so if you were assigning to something other than `_` you'd get a warning that you're ignoring the `Pending` path. The Clippy lint is only for `_` which Rust considers a use by default.

Re: We found a bug in the hyper HTTP library

#48

Post by pseudony »

c0balt wrote: post #34
pseudony wrote: post #33

So “fearless concurrency” still only happens when one just decides to not be afraid… :)

This does not appear to be a concurrency bug though?

“ a race condition that occurred only under specific conditions — in the hyper library”

Re: We found a bug in the hyper HTTP library

#49

Post by pwdisswordfishq »

Twey wrote: post #43
pwdisswordfishq wrote: post #35

Earlier quoted context omitted.

Ehh, easy fix #[allow(clippy::let_underscore_untyped,clippy::let_underscore_must_use)] let _ = self.poll_flush(cx)?;

I said ‘flagged’, not ‘fixed’ :) You can always write the wrong code if you want it enough. But hopefully a warning would have prompted someone to think harder about this flow.

But "let _ =" is already an explicit suppression of a must-use warning. Where does this arms race of "no, I really know what I am doing, compiler" versus "no, this really looks like a mistake, programmer" end?
Post reply on HN