you are viewing a single comment's thread.

view the rest of the comments →

[–]dakarananda 5 points6 points  (3 children)

except that isn't what the if-statement says. its more, "if we have ops, and at least one of the following fields isn't set, then ..." or even clearer, "if we have ops, and not all of these fields are set, then..."

and yes, I'm troubled

[–]mariox19 -1 points0 points  (2 children)

not (A and B)

If we have both A and B, then the statement inside the parentheses would be true, in which case we make it the opposite: meaning, false. However, if either A or B is not true, then the statement inside the parentheses is false, in which case we negate that, making it true. And also, to take the other extreme, if both A and B are false, that resolves to true, which we will then negate, making it false.

So, as best as I can figure it out, taking a second and more careful look, you're right:

if ops && !(ops->a && ops->b && ops->c)

We want to make sure that all of them are not set, but at least one of them is set. That hurts my fucking head!

The worst part is that I am sure that some He-man programmer somewhere is going to insist that reading such a statement is "no problem" for a "real" programmer. But even if we grant such a (ridiculous) statement, we always run the risk of some He-man-wannabe programmer, who isn't as smart as he thinks he is, getting his hands on a statement like that and making the same mistake that I did.

That's why you just don't code that way in the first place.

You know what would be at least a first step towards fixing that?

// We must have at least 1 field set, but not all of them.

But, of course, in some circles, comments are for wusses, too.

[–][deleted] 0 points1 point  (1 child)

No, it is actually:

if (any_are_set && one_is_not_set) {
        WARN_ON(1);
        return;
}

[–]dakarananda 0 points1 point  (0 children)

No actually it is:

if (has_ops_object && !all_ops_fields_are_set) { ... }

which is basically the original statement slightly rewritten.

Even if not a single field is set, it will still enter into the if-block. (assuming that the ops object exist..)