A Valuable & Comprehensive Firmware Code Review by Codethink

Did you know that Codethink is able to review software code independently for your projects?

That is precisely what Ben Brewer (Head of Embedded at Codethink) did successfully for another of our clients, ASL Holdings Ltd recently, following a referral through our Microchip Design Partnership.

Codethink’s decades of collective software expertise has qualified us to independently review and scrutinise code, enabling us to provide sound advice on how to improve code before the code, quite often within products, ultimately hits the market. Additionally, we often provide guidance to our clients on how to ease the burden of long-term software maintenance during code review analysis.

Whilst writing and reviewing code is standard practice for our team at Codethink, not all companies are able to follow this methodology for various reasons. We’ve often found that an independent review provides not only a sanity check on the code but higher confidence and quality assurance for many of our clients.

Why Do We Bother With Code Review?

Code reviews have until recently always been a controversial issue especially amongst programmers, on the one hand a review from someone with more experience and expertise can be useful, but often-times rushed code reviews can lead to low-quality nitpicking and can become a hindrance to the developer, or worse: A code review can easily become a battle of ego’s and wills.

This article will discuss code reviews in general but will reference a recent in-depth code review we performed for ASL Holdings Ltd over the course of 2 weeks.

What is the point of this code review?

When performing a code review we should always ask ourselves what the point of the review actually is, and try to answer some basic questions.

Who is the target audience for the review?

A junior developer may request a review as a learning experience, and would expect any response to be as verbose and educational as possible.

A senior developer may request a review as a sanity check and will often expect a review to be as technically in-depth as possible to catch/discuss potential issues.

A manager may request a review either directly or as part of a company process, in this case they will need a high level summary to provide them feedback on the overall code quality on the project and as a feedback mechanism to feed into any personal development programs.

A company may request a review as part of an evidence chain for safety/security certification or approval, such a review would be expected to be as formal and verbose as possible as it will need to be referenced in the future.

In practice all code reviews are a mixture of the above, and before undertaking a code review it’s important to gauge the audience as the wrong review focus or format could lead to a bad code review.

In our work with ASL Holdings we were working to review their power monitoring devices for the purposes of both safety and quality. Since we knew the audience for this review from the start, it became clear that this review would need to be very technically in-depth and formal.

What does a successful review look like?

Once we know the target audience it becomes easier to determine what results are expected. In some cases this may be covered by company process or by the request for the review, but it’s often worthwhile to directly ask the main stakeholders what they expect to get out of a code review.

Typical expected outcomes from a code review can vary a lot, and include:

• Fixing known bugs
• Finding unknown bugs
• Describing potential maintenance issues
• Simplification of the codebase
• Providing optimizations
• Ensuring common conventions/styles are used
• Education/Feedback

For a longer form review it’s often worthwhile to state the intentions of the review up-front as this will prepare the reader for what to expect from the review.

Where to Begin?

For a small code review it can be obvious where to begin if there’s relatively little to tackle, but this question can be more tricky to answer for large projects/commits.

In our work with ASL Holdings we were reviewing a mature codebase with a large number of C source and header files and no prior knowledge of the codebase. The lack of prior knowledge can be an important factor in providing an unbiased review and may be a requirement for safety, security or quality processes.

A larger code review should always begin with a discussion with the original author where possible as they will be able to point out any areas of concern, any known issues or any concerns they have about the code in general.

When digging into a large codebase it’s good to have a number of entry points as this can focus the mind, and once a high-level review of these areas is written the experience should allow for a wider understanding of the codebase.

Due to the requirements for the ASL Holdings code review it became obvious early on that the only acceptable outcome would be to review every single line of code.

Reading large quantities of code may seem daunting at first, however most codebases will have a relatively consistent style whether due to a specific code style being followed or just due to the limited number of authors. The important thing when tackling a large quantity of code is to keep very brief notes so as not to break your flow when reading through the codebase.

For the ASL code review we kept very brief notes indicating line-number and a couple of words to describe each issue; by keeping minimal notes this allowed us to read through the codebase quickly without interruption. When it came time to write the formal review; using only these notes it was possible to work backwards and focus on specific parts of the codebase in the review itself.

Reading through the codebase first also allows us to determine any recurring issues, and get a picture of how much time we’ll have available to dig into each review point.

Understanding how much time we had to focus and think about specific issues was important as we could plan to provide the most in-depth feedback in the time given. This allowed us to provide very in-depth feedback for the most critical point by referencing appropriate technical documentation and providing code improvement snippets.

Where to End?

By determining the audience, stakeholders and expectations of the review, it becomes easier to determine where the end-point should be.

Typically a code review shouldn’t end until you have a full understanding of the reviewed codebase such that you can read and understand every function/file. Where this is not the case, the level of depth should be clearly mentioned in the code review.

Often for shorter code reviews which may be part of a company commit process there’s a temptation to review briefly and accept a change, however if it’s not clear that this has been done, then bugs and technical debt can easily build up unnoticed.

In the case of our review for ASL Holdings we knew that a review of the entire codebase was required for quality and safety purposes, so it became our obligation to review with a full understanding.

While we had gathered requirements up-front to determine what a successful review might look like, it was important to leave some time for feedback to the code review as the output in our case was a formal document.

Conclusion

In our work with ASL Holdings we were given two weeks to review a PIC codebase for a couple of projects based on a PIC18 microcontroller. This review was referred to us through our partnership with Microchip Technologies due to both our expertise in providing embedded solutions and due to ASL Holdings being a local UK based company.

We provided an in-depth 57 page review for a codebase we had no prior experience of within 2 weeks and managed to provide bug fixes, code improvements and RAM/ROM optimizations which allowed space for ASL Holdings to consider adding additional features without increasing BOM cost.

Special Thanks

Special thanks to Shaun Clements (Principal Design Engineer at ASL Holdings Ltd), Allan Sydney (CTO at ASL Holdings Ltd) and Andy Walton (Head of Operations and Quality at ASL Holdings Ltd) for this opportunity to work with ASL Holdings Ltd. It was a pleasure to work with you all.

ASL is connecting the world one device at a time, providing premier and innovative IoT connected technology solutions. Purchased by Halma plc in March 2013, ASL is now part of HWM-Water Ltd who are a global leader in the manufacture of data loggers and leak detection products for the water industry, within Halma’s Environmental & Analysis sector. For more information please visit their website http://www.aslh.co.uk

Testimonial

“Codethink provided us with a very valuable and comprehensive report after having independently reviewed our firmware code. After the team assessed the report internally and implemented the recommendations, we felt fully confident to release our products into the field. The service, communication and support from start to finish was excellent. Shaun and Ben worked together and understood each other very well, and all work was completed within the agreed time and budget. We would highly recommend using Codethink for all things software related and especially in our case where they needed to have a very good understanding of the firmware. A sanity check is always a good shout too. We would not hesitate to use Codethink again and they proved to be a great Microchip Design Partner. Thanks again” Andy Walton (Head of Operations and Quality at ASL Holdings Ltd).

Our Services

If you’d like Codethink to undertake an independent code review on behalf of your project or require any software engineering or consultancy support, please feel free to get in touch with us, we’d be delighted to help:

sales@codethink.co.uk or +44 161 660 9930

In a nutshell, Codethink provides consultancy and software engineering services to international-scale customers. We help to design and deliver systems and software at all scales, from dedicated microcontroller/DSP/FPGA solutions to embedded medical devices, in-vehicle systems, network and cloud infrastructure. Codethink is one of the few firms in the market with the expertise to reliably deliver complete custom Linux systems and Open Source solutions for critical devices and services. For more information, please visit our website https://www.codethink.co.uk