Hacker News Reader

Comment by harrytuttle | Hacker News Reader

And this is why you write it like this when you use C:

   (0 == current->uid)

Rather than

   (current->uid == 0) // or (current->uid = 0) in this case.

Impossible to make an assignment to a numeric lvalue. Easy to spot, easy to audit in a diff.

View on HN

> And this is why you write it like this when you use C:

Sure, if you are in habit of releasing code that compiles with warnings.

This sort of pointless code twisting is intensely annoying. It solves a rare problem with a wrong tool and in a very intrusive way.

Zero is constant. You compare to it.

Huh?

   $ cat test.c
   int main(int argc, char *argv[]) {
       if (0 == argc)
           return 1;
       return 0;
   }
   $ CFLAGS="-Wall -ansi -pedantic" make test
   cc -Wall -ansi -pedantic    test.c   -o test

Nope, no warnings there.

Right, but if you compile with warnings as errors, then assignment within a condition won't compile, which prevents the issue.

I'd much rather go that way than write code that reads less like a human wrote it.

What compiler flag is that? Genuinely interested. Wall,pedantic,ansi don't trigger it. I've tried the following and I don't get a warning or error:

   $ cat test2.c
   int main(int argc, char *argv[]) {
       if (argc = 0)
           return 1;
       return 0;
   }
   $ make test
   cc -Wall -ansi -pedantic    test.c   -o test

GCC version: gcc version 4.7.2 (Debian 4.7.2-5)

  huh@px:/tmp$ cat a.c
  int main(int argc, char *argv[]) {
         if (argc = 0)
             return 1;
         return 0;
     }
  huh@px:/tmp$ gcc -Wall a.c
  a.c: In function 'main':
  a.c:2: warning: suggest parentheses around assignment used as truth value

Doesn't do that for me - what GCC ver and environment. Very odd! Digging in docs. Thanks for info.

It's some old-ish Debian on x86. GCC is 4.3.2.

But this is a such commonly-recognized pitfall that I actually don't know a single modern production compiler that does not generate this warning.

(edit) Just checked 4.7.1 and it generates the warning.

Thanks. I think this must be a config issue on this box. It makes me wonder what other warnings it is not telling me about.

Notice that you cat test2.c and then compile test.c.

Well spotted! Thanks for pointing this out!

I will now go and hit myself with a LART for a bit to remember not to do that again.

However the same issue happens in a lot of C-like languages, and not all of them have compilers that inform you of errors. But the habit works in all of them.

There are two different ways to catch this class of bug. Both have value.

I believe they mean that

    if (foo = 0) ...

generates a warning like this:

    warn.c:2: warning: suggest parentheses around assignment used as truth value

which you should be paying attention to, in favour of yoda-comparisons in this particular holy war :)

I would but if you see the rest of the thread, my gcc isn't raising this as a warning!

That in itself is sign that a mistake can be made without noticing.

The code in question (see the LWN link elsewhere in this discussion) actually had parens around the expression (in a context where they looked "normal") and wouldn't have generated a warning.

That said, I agree with huhtenberg above that twisting the language conventions around to deal with this is never going to fix anything. Subtle code remains subtle in all languages, and subtle code is where security bugs lie. You can't fix "subtle" with a rulebook.

Or run a linter or static checker which disallows assignments in conditionals as part of your build and/or checkin process. They'll catch this and other fun C "gotchas" without relying on human vigilance.

That's the failsafe (one I believe heavily in as well). But getting it right up front is still the best approach.

Makes one appreciate languages like Pascal which differentiate between the equality operator '=' and the assignment operator ':='.

I wonder why the decision was made in C to use '=' and '=='? I'm betting it has something to do with the laziness of the programmers :).

The problem is not in == vs =, it's in allowing assignment as an expression. Go fixes this, for example, by now allowing them: if a = 0 {} gives compile error.

The problem is that == and = look similar. Banning assignment as expression only makes the language less expressive. Common Lisp fixes this, by having assignment and comparison look completely different: (setf variable value) vs (eql variable value)

Bit of a bummer, I often find using assignment as an expression useful, like if (x = y as String) == null or such in c#. I get the feeling Go sacrifices a bit too much freedom.

Or you could also blame it on not having branch statements which enforce the use of a boolean type.

Edit: Actually, forget I said this. It's a dumb statement. 'if(a = true)' would always pass, regardless of the value of 'a'.

I'm not a professional programmer by any stretch, but wouldn't this problem completely go away if the C and C++ compilers detect a single = inside an if statement and outright refuse it. Either that or automatically substitute the assignment with checking for instances inside the if-condition.

The reason I'm asking is, because I don't know if there's ever a case where you'd want to assign right inside the if-condition. Is there?

There can be:

    if (Foo *foo = getFoo()) { // getFoo returns null if there is no foo
        // There is a foo
    }

However, this problem is basically entirely addressed by compiler warnings, which generally ask you to add an extra set of parens for this case.

You don't do that because it makes the code less readable. If you codebase is really big you have to read the code all the time and it takes more brain power to translate your version. Also gcc has a warning vor assignments in if, which are imho a bad habit.

Thanks for this.

I don't actually use C, although I know a few languages with C-like syntax (Java, JS, AS2.0) and I'm learning Go & planning to learn D.

This small change will be really helpful in all these languages. Even if I never make this error again it will help stop people who alter my code from making this error.

Perhaps they should teach it in more programming textbooks, as I've not come across it in anything from HeadFirst to online tutorials to Deitel & Deitel.

FYI, sometimes called "Yoda condition". I _think_ I was taught to do this when I learnt C (~20 years ago). Wikipedia indicates that the name at least has been around since 2005.

It's not necessary in Java (though it's still a common habit). Assignment to a non-boolean is impossible in an if statement in Java, and you can make the compiler warn you about accidental assignments.

Still, assigning a boolean rather than comparing it is still possible and, possibly, a source of bugs.

This is the same procedure (or rather should be) in PHP. Also, better to switch to === or !== for exact comparisons since that also checks type.

(I'll leave out the other warts of PHP since that's been discussed here and elsewhere ad nauseum)

Good point i always wondered why people write it like that it makes it harder to read.

I see what you're saying, but (0 == current->uid) still makes my eyes bleed

I agree with you, but it works and it's saved me hours and hours of crying in front of gdb.

Please don't do that...

That what? Why not?

Writing (0 == a) instead of (a == 0) to check a's equality with 0.

It sacrifices readability. I'm sure there are tools (if not one can easily write one) that warn you for accidental assignment when comparison was meant instead.

Moreover, the (0 == a) trick only works for constants (which you can't assign values to, hence the compiler will complain), but not for variables. I.e. you can still do if (a = b) accidentally, when comparison was intended.

Reply on news.ycombinator.com