Article 602SR CodeSOD: Mostly Okay

CodeSOD: Mostly Okay

by
Remy Porter
from The Daily WTF on (#602SR)

Taffer is the team lead on a team making security products. As such, they have very strict policies about how they write their code, they have very thorough code review systems, and they also have automated tests for everything.

And yet, things can still slip through.

Taffer submitted this change. It passed two code reviews. It didn't cause any unit tests to fail. It made it into the main branch, and sat there for two months. A team of very experienced, very senior developers didn't catch this glitch until a new hire happened to notice it during a review for something else.

status_t retval = internal_check_struct_integrity(input_struct);if (retval != OK) { return OK;}

It's a simple mistake to make, and would hopefully be a simple mistake to catch, but alas, it turned out to be hard. Obviously, if the integrity check fails, we don't want to return OK.

It's not all bad. It never made it into a released product, and it has additional benefits, as Taffer explains:

On the plus side, we have a great example of "anyone can make mistakes" to show new hires and interns when we're introducing them to our code review regime.

proget-icon.png [Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more. TheDailyWtf?d=yIl2AUoC8zA
External Content
Source RSS or Atom Feed
Feed Location http://syndication.thedailywtf.com/TheDailyWtf
Feed Title The Daily WTF
Feed Link http://thedailywtf.com/
Reply 0 comments