Hacker News new | ask | show | jobs
by WhiteSage 2145 days ago
From the article comment section:

> this is not a mistake.

> Assume that the coder meant == 0 what is he trying to enforce. If these 2 bits (_WCLONE and _WALL) are set and your are root then the call is invalid. The bit combination is harmless (setting WALL implies WCLONE [...]), and why would you forbid it for root only.

2 comments

> ...code change in the CVS copy that did not have a pointer to a record of approval. Investigation showed that the change had never been approved and, stranger yet, that this change did not appear in the primary BitKeeper repository at all...

I'll attach this here for people who read the article too quickly and think it may, somehow, have been a bug. This code was a very deliberate attack.

This is also very relevant comment:

> In addition, parentheses were not required for the final comparison. This was done to prevent compiler warnings. This looks deliberate.

I would put parentheses here, I never like mixing logical operators with other types (or even different types of logical operators). While it's of course entirely redundant here, it also makes the code easier to read IMO.

I think the parent's point is more convincing: why make this check only for root in the first place?

The parentheses are required if == is changed to =.

== has a higher precedence than &&, but = has a lower precedence.

   a = b && c && d
means

   a = (b && c && d)
Sure, my point was that even with the proper == comparison I'd still write the (now redundant) parens because I find it more readable that way.

Actually in languages like Rust with type inference that make it cheap and non-verbose to declare intermediate values I tend to avoid complicated conditions altogether, I could rewrite the provided expression like:

    let invalid_options = (options == (__WCLONE|__WALL));
    let is_root = (current->uid == 0);

    if invalid_options && is_root {
        // ...
    }
One might argue that it's overkill but I find that it's more "self-documenting" that way. I find that the more experienced I get, the most verbose my code becomes. Maybe it's early onset dementia, or maybe it's realizing that it's easier to write the code than to read it.

Of course you can do that in C as well but you have to declare the boolean variables, and in general it's frowned upon to intermingle code and variable declarations so you'd have to add them at the beginning of the function so it adds boilerplate etc...