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

Google broke a conditional statement that verifies passwords on Chrome OS

arstechnica.com

Post reply on HN
Print view View original

211–220 of 276 posts

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#211

Post by d110af5ccf »

gpderetta wrote: post #210
d110af5ccf wrote: post #161

Earlier quoted context omitted.

This. Author chose two spaces per indent and proceeded to nest blocks 5 levels deep? Guess I have to run it through a code formatter now just to read it. Alternatively, author chose eight spaces per indent and proceeded to nest things? My tiled windows only have ~100 columns, thanks so much for breaking my workflow with your poor choices.

But why not? These days reformatting on the fly is fast and easy. Especially if you enforce formatting conventions on commit (so you can automatically reformat into a canonical version).

Because typically code will be read far more than it is written and by far more people than contributed to it. In many cases I won't have the relevant tooling installed let alone know how to use it because I don't use that language myself. (For example, a library written in Rust that exposes C bindings whose prebuilt artifact I might download and link against.)

That being said, I 100% wholeheartedly approve of enforcing code formatting (among other things) on commit. Pre-commit hooks and linters are both awesome.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#212

Post by pornel »

timvisee wrote: post #203

Rust would have prevented this.

Rust's Option can't be simply dereferenced causing UB, so `val.is_some() & val.kaboom()` doesn't compile.

Rust doesn't have implicit conversion between booleans and integers, so a problematic `true & 2` does not compile either.

Rust does allow `true & true`, but booleans are strictly 0 or 1 in value, so you wouldn't get a wrong result, apart from eager evaluation. There's needless_bitwise_bool clippy lint for it.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#213

Post by timvisee »

pornel wrote: post #212
timvisee wrote: post #203

Rust would have prevented this.

Rust's Option can't be simply dereferenced causing UB, so `val.is_some() & val.kaboom()` doesn't compile. Rust doesn't have implicit conversion between booleans and integers, so a problematic `true & 2` does not compile either. Rust does allow `true & true`, but booleans are strictly 0 or 1 in value, so you wouldn't get a wrong result, apart from eager evaluation. There's needless_bitwise_bool clippy lint for it.

Yes! While the other way around an accidental `&&` when `&` would have failed as well.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#214

Post by pornel »

inshadows wrote: post #183
formerly_proven wrote: post #44

Earlier quoted context omitted.

The code is: key_data.has_value() && !key_data_->label().empty() key_data is probably something like an std::optional, so short-circuit evaluation guards the dereferencing of the optional. If you write this with a binary and, no short-circuit evaluation can happen, so undefined behavior ensues. I wanna point out that std::optional, in the typical C++ stance, has a "I know what I'm doing" API and a "Humans make mistak…

Where is the undefined behavior? AFAU, UB is a compile time concept. As other commenter has asked, how can the compiler infer that has_value() call has any relation to key_data_ non-NULL-ness? EDIT: > optional->... is UB if the optional is empty I understand what you mean: dereferencing optional is undefined if has_value() is not checked for true in the same thread before. But that undefined behavior happens at runti…

UB is not an event that happens. UB is a rule that allows compilers to assume that situations defined as UB never happen.

It's UB to access an optional that has no value, so if the compiler sees optional accessed unconditionally, it may assume the optional must have had a value, and delete all code that contradicts this conclusion.

Compilers don't do this out of spite. It's an optimization that removes redundant checks (e.g. when code in an inline function checks args != null, but you use the function with obviously non-null args).

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#215

Post by xupybd »

gpderetta wrote: post #210
d110af5ccf wrote: post #161

Earlier quoted context omitted.

This. Author chose two spaces per indent and proceeded to nest blocks 5 levels deep? Guess I have to run it through a code formatter now just to read it. Alternatively, author chose eight spaces per indent and proceeded to nest things? My tiled windows only have ~100 columns, thanks so much for breaking my workflow with your poor choices.

But why not? These days reformatting on the fly is fast and easy. Especially if you enforce formatting conventions on commit (so you can automatically reformat into a canonical version).

Automated reformats cause big problems in version control.

Every line appears to come from the reformat commit. So it's hard to see when the codes function was changed.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#216

Post by gpderetta »

d110af5ccf wrote: post #211
gpderetta wrote: post #210

Earlier quoted context omitted.

But why not? These days reformatting on the fly is fast and easy. Especially if you enforce formatting conventions on commit (so you can automatically reformat into a canonical version).

Because typically code will be read far more than it is written and by far more people than contributed to it. In many cases I won't have the relevant tooling installed let alone know how to use it because I don't use that language myself. (For example, a library written in Rust that exposes C bindings whose prebuilt artifact I might download and link against.) That being said, I 100% wholeheartedly approve of enforc…

If you do not have the relevant tooling you can still read the canonical formatting.

If you want custom formatting, I don't think that expecting your editor to reformat on the fly is a big ask. At the end of the day that exactly what tab proponents expect.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#217

Post by gpderetta »

xupybd wrote: post #215
gpderetta wrote: post #210

Earlier quoted context omitted.

But why not? These days reformatting on the fly is fast and easy. Especially if you enforce formatting conventions on commit (so you can automatically reformat into a canonical version).

Automated reformats cause big problems in version control. Every line appears to come from the reformat commit. So it's hard to see when the codes function was changed.

you format as part of the commit, not as a separate commit.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#218

Post by d110af5ccf »

gpderetta wrote: post #216
d110af5ccf wrote: post #211

Earlier quoted context omitted.

Because typically code will be read far more than it is written and by far more people than contributed to it. In many cases I won't have the relevant tooling installed let alone know how to use it because I don't use that language myself. (For example, a library written in Rust that exposes C bindings whose prebuilt artifact I might download and link against.) That being said, I 100% wholeheartedly approve of enforc…

If you do not have the relevant tooling you can still read the canonical formatting. If you want custom formatting, I don't think that expecting your editor to reformat on the fly is a big ask. At the end of the day that exactly what tab proponents expect.

> If you do not have the relevant tooling you can still read the canonical formatting.

Which is often unnecessarily difficult for whatever reason, hence my initial comment. Given that tabs already exist there's literally no reason not to use them other than to intentionally spite any future readers who don't agree with your preference for indentation width.

> that exactly what tab proponents expect

Because that capability is built into even the most primitive of text editors since forever. Per-language formatting, on the other hand, is most certainly not.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#219

Post by d110af5ccf »

xupybd wrote: post #215
gpderetta wrote: post #210

Earlier quoted context omitted.

But why not? These days reformatting on the fly is fast and easy. Especially if you enforce formatting conventions on commit (so you can automatically reformat into a canonical version).

Automated reformats cause big problems in version control. Every line appears to come from the reformat commit. So it's hard to see when the codes function was changed.

I believe what's being described is on the fly (ie uncommited) reformatting to suit individual reading preferences. Additional enforced autoformatting at the time of commit ensures that all version controlled code always conforms to the chosen style. Thus there are never any reformat commits unless the chosen style is modified.

Re: Google broke a conditional statement that verifies passwords on Chrome OS

#220

Post by inshadows »

pornel wrote: post #214
inshadows wrote: post #183

Earlier quoted context omitted.

Where is the undefined behavior? AFAU, UB is a compile time concept. As other commenter has asked, how can the compiler infer that has_value() call has any relation to key_data_ non-NULL-ness? EDIT: > optional->... is UB if the optional is empty I understand what you mean: dereferencing optional is undefined if has_value() is not checked for true in the same thread before. But that undefined behavior happens at runti…

UB is not an event that happens. UB is a rule that allows compilers to assume that situations defined as UB never happen. It's UB to access an optional that has no value, so if the compiler sees optional accessed unconditionally, it may assume the optional must have had a value, and delete all code that contradicts this conclusion. Compilers don't do this out of spite. It's an optimization that removes redundant chec…

Yeah, I know what UB is, and that's why I'm saying that this is not UB. Have you even read what I wrote?
Post reply on HN