10 Questions to Consider when Reviewing Code

There are 10 questions every developer should ask themselves when reviewing their code that can help find potential bug-ridden areas.

Over the years, as I have reviewed and analyzed embedded software, I have noticed a few common issues that even to this day remain. Issues that no matter the company size or how mature the development process always seem to pop-up. In order to help alleviate these common issues, there are 10 questions every developer should ask themselves when reviewing their code that can help find potential bug-ridden areas.

 

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 issue that I continue to see from both green and senior engineers alike. The worst case I have seen had more than 1,000 warnings! The most disturbing case had just one: a simple implicit cast warning of an unsigned integer into a float.

 

2. Are there any blocking functions?

One primary purpose for 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.

code, flash, ESC Boston 2017, Embedded Systems Confererce, microcontroller, MCU
Figure 1 – Blocking Button Debounce

 

3. Are there any potential infinite loops?

Who in their right mind would put an infinite loop into their code on purpose? (Excluding of course those needed in tasks or the application’s main loop.) Yet, there is a lot of example code on the web and from microcontroller vendors that exhibit an infinite loop failure behavior. For example, code that writes data to flash or EEPROM will typically monitor a hardware flag for completion. Example code will create a while loop on the flag to reach a certain state before allowing code execution to continue. If the hardware fails and the flag never sets, the code becomes stuck in an infinite loop! An example of this can be seen in Figure 2.

code, flash, ESC Boston 2017, Embedded Systems Confererce, microcontroller, MCU
Figure 2 – The Infinite Loop upon Failure

One way that this infinite loop failure can be remedied is to have the loop monitor a system tick or to limit the number of times that the loop can execute before finally deciding that an error has occurred. These remedies allow for error handling to be built into the system if the hardware fails. Despite popular belief, hardware (and software) does fail.

 

4. Should this function parameter be const?

Programmers tend to not use const as much as they should especially when it comes to function parameters. Declaring a passed function parameter as const is a great way to protect that variable from being accidentally modified within the function. Why leave it to chance that a future developer will realize that they shouldn’t be modifying that system-critical variable but should only be using it?

 

5. Is the code’s cyclomatic complexity less than 10?

Monitoring a function’s cyclomatic complexity metric is a great way to help limit how complex a function gets. This metric directly relates to the minimum number of test cases that need to be performed on a function to test every branch. Not only that, the metric really tells how much a developer needs to keep track of in their mind while they are writing or modifying a function. Since most humans can only keep track of seven to nine things at once, keeping cyclomatic complexity less than 10 is a good bet to help keep bug rates down. (Check out “Limiting Function Complexity is a Requirement.”)

 

6. Has extern been limited with a liberal use of static?

The C language defaults a variable’s scope to extern. This default is implicit; a variable declared in a module without the use of static has a big invisible extern sitting in front of it. The only way to get rid of that invisible extern is to place a visible static in front of the declaration. This practice has the added benefit of making the variable local in scope, aiding in data hiding and encapsulation. The most common place to look for implicit externs is module level variable declarations.

 

7. Do all if … else if … conditionals end with an else?

The use of a default case in a switch statement should be mandatory. Static analysis tools will complain if a default case isn’t present. A developer can easily see that if a conditional warrants the use of a switch statement with various cases, there may be a case that is unexpected or overlooked that should have a default end-all case. This applies to if … else if … conditionals as well. If two or more conditions are to be checked for, what should be done if none of these cases meets the current condition? The final else in the statement acts just like default case in a switch statement.

 

8. Are assertions and/or input/output checks present?

One of the very early lessons that I remember from programming classes in high school was that inputs and outputs to functions should be checked to ensure that they are valid. Unfortunately, despite this teaching, embedded software developers tend to ignore this computer science wisdom. Embedded software developers should be sprinkling their code with assertions to validate that their assumptions about the program’s behavior at certain points is correct. Boundary checks should be performed on inbound and outbound data. Remember the old saying “garbage in, garbage out?”

 

9. Are header guards present?

A header guard is a simple macro that ensures that the header file is not included more than one time within a translational unit. The guard prevents double inclusion of the #include directives. Not including the header guards can result in some very strange static analysis behavior. More importantly, using a guard prevents multiple definition errors.

 

10. Is floating point mathematics being used?

The use of floating point mathematics can be a sticky subject in embedded systems. Resource-constrained microcontrollers often do not include a floating point unit (FPU). This absence means that the processor has only one means of performing floating point calculations: using a library function. Library functions for floating point math are usually slow and inefficient, they don’t necessarily have deterministic behavior, and they can cause code size to balloon. For these reasons developers should carefully consider when to use floating point within a microcontroller. They should also perform additional testing and should consider alternative methods such as look up tables, scaling, and fixed point math.

 

Conclusion

Each programmer has their own unique view and insights into embedded software development. The C language that we have all come to love and hate is so encompassing that there is always something to learn. Despite the many insights and varying degrees to which code reviews and analysis tools are used, errors persist. These 10 questions address common errors and misconceptions with developing embedded software that should be checked with every code review.

 

ESC, Embedded Systems Conference, BostonBootloader Design Techniques for Microcontrollers. 
Remotely updating firmware is a critical for any embedded system and even more so for an IoT device. Bootloaders are often added along side application code to facilitate firmware updates but very few developers truly understand how to build one robustly. Join Jacob Beningo as he examines the fundamental challenges facing software engineers in performing updates in a secure and safe manner during his half-day tutorial, "Bootloader Design Techniques for Microcontrollers" at ESC Boston 2017, May 3-4. Register today!

 

 Jacob Beningo is an embedded software consultant who currently works with clients in more than a dozen countries to dramatically transform their businesses by improving product quality, cost and time to market. He has published more than 200 articles on embedded software development techniques, is a sought-after speaker and technical trainer and holds three degrees which include a Masters of Engineering from the University of Michigan. Feel free to contact him at jacob@beningo.com, at his website www.beningo.com/, and sign-up for his monthly Embedded Bytes Newsletter.

 

Comments (3)

Please log in or register to post comments.
By submitting this form, you accept the Mollom privacy policy.
  • Oldest First
  • Newest First
Loading Comments...