Skip to content

Instantly share code, notes, and snippets.

@dbrady
Created September 28, 2016 18:54
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save dbrady/27f7f788a513d18a83bbdc859fb8e2ba to your computer and use it in GitHub Desktop.
Save dbrady/27f7f788a513d18a83bbdc859fb8e2ba to your computer and use it in GitHub Desktop.
EDIT: Was gonna tweetstorm this, then decided to just gist it. Enjoy. ;-)
Tweetstorm about deceptive code incoming.
First Point: People believe comments over the code, even if the code is obvious: x=1; /* put 2 in x */ <-- people believe the 2
Second Point: People maintain code without maintaining (or sometimes even reading) the comments for that code
Third Point: Programmers love to optimize. Makes us feel smart. We think it makes us LOOK smart. But that way lies madness...
So. Dateline: Salt Lake City, 2002. I find this piece of assembly code: "XOR EAX, EAX ; PUT 1 IN EAX"
XOR any number with itself, you get 0. So the comment here was wrong, but if you don't know the XOR trick, it's outright deceptive
If you DO know the trick, it's still distracting and makes you stop and question your sanity for a moment
So I tracked down the commit history for the commit, and here's what happened (exit tweetstorm here if you don't want full history)
1. Original programmer decided that we would make a decision in the code based on true (1) or false (0), and it started out true
2. So they wrote "MOV EAX, 1", which stores a 1 in EAX
3. Somebody decided to comment this line with "; PUT 1 IN EAX".
4. Later, somebody decided we should switch to using the "false" case in the code, so they changed it to "MOX EAX, 0"
5. BUT THEY LEFT THE COMMENT. This is deceptive enough already, because it's just plain wrong: "MOV EAX, 0 ; PUT 1 IN EAX"
6. On pre-286 computers, it takes 2 clock cycles to move an immediate value into a register, but only 1 to XOR. Enter the optimizer!
7. The code got changed to "XOR EAX, EAX". Fun fact: ALL post 286 computers take 1 clock to MOV, and ONLY post-286 have EAX register
8. This was literally a useless optimization. The author probably defended it on the basis that XOR is a well-established idiom.
9. Steve McConnell: "When you optimize without measurements, the only thing you know for sure is you made the code harder to read."
10. And, of course, the optimizing programmer ALSO did not maintain the comment, leaving "XOR EAX, EAX ; PUT 1 IN EAX"
11. So they made the code "clearer" and "more idiomatic" but ALSO left a comment that said the exact opposite of what they did
12. Which means people who don't know the idiom will be deceived by the comment, but MY point is: also people who DO know
13. I only spotted it because I saw the XOR EAX, EAX and smugly thought to myself "Ah, I know that optimization! It--wait, what?
End tweetstorm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment