10 Questions to ask during code reviews

April 29, 2015 // By Jacob Beningo
Over the years I’ve noticed a number of common “gotchas” when reviewing code. They're there no matter what the size the company or how mature the development process (and I have had the opportunity to review software for companies ranging from those with strict and bureaucratic processes to those that are more shoot-first-aim-later).

In order to help alleviate these common issues, there are 10 questions that can be asked while reviewing C code to help find potential bug-ridden areas.

Question #1 – Does the program build without warnings?

Code cannot be loaded on a target without a successful compile. A successful compile involves the programmer diligently removing any syntax errors in order to make the compiler happy and have it create an output file. But a compiler can build an application without error yet still find other anomalies, such as an implicit cast, that it reports as a warning. A truly successful compilation of a program, then, should involve not just compiling with zero errors but also with zero warnings. This definition of successful compilation may seem obvious, but failure to resolve warnings is an error that I continue to see from both green and senior engineers alike. The worst case I have seen had more than 100 warnings! The most disturbing case had just one: a simple implicit cast warning of an unsigned integer into a float.

Question #2 – Are there any blocking functions?

One of the primary purposes of a microcontroller (MCU), in my opinion, is to be able to handle real-time events. MCUs should be able to handle those events in a very deterministic manner that can be measured and proven. Yet one of the common mistakes that I often see is that a driver or some application code segment is written such that it enters a loop or calls a delay function for an extended period of time. But a loop or delay prevents any other code from running on the processor, potentially compromising determinism. Button debounce functions are a notorious example. Instead of polling the GPIO pin or setting up an interrupt, many debounce implementations read the pin, then enter a delay of 20, 30, maybe even 40 milliseconds before reading the pin again. Take a look at Figure 1 for an example. In an environment without an RTOS this delay is the death of a real-time system.


Figure 1 – GPIO debounce by blocking