CodeSOD: Mostly Okay

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.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

This post originally appeared on The Daily WTF.

Leave a Reply

Your email address will not be published. Required fields are marked *