Friday, October 20, 2006

Code Review Guidelines that matters Embedded Engineers

To provide the customer with perfect code and to reduce Testing phase time, I used do the following checks for each function:

  1. Unused Function arguments and Variables removed.
  2. Variables compared for Negative value declared as Signed.
  3. No possibility for Infinite loop on failure of the Conditional statements.
  4. No possibility for Variable or Function arguments left Uninitialized on failure of the Conditional statements.
  5. NULL inputs handled.
  6. Validity check of Function inputs done (For APIs, it is must).
  7. If the function is local, it is declared as "static" and prototype is added in top of the local file. Else, the prototype is added in the header file.
  8. Macros are defined with correct value.
  9. Function is Commented well.
  10. Returns proper Error Codes.
  11. Release of allocated resource is taken care.
  12. In case of erroneous return, the function is brought back to the Stable state before Returning error (Release allocated resources, etc,.).
  13. memcpy or memset are in terms of bytes.
  14. Endian handled.
  15. Boundary Alignment handled.
  16. If you`ve called NULL returning functions, did you check for NULL result?
  17. Before calling the Callback, did you check for NULL?
  18. Before assigning value using pointer, did you check for NULL pointer?
  19. Architecture compatible? For example, the variable that involves in 32 bit operation has to be declared as UW (means, 32 bit in all architectures). If other variable types are used, it has to be typecasted to force the compiler to use 32 bit. For example, (UW)byte[n] <<8.
  20. If any Flag is used, is it reset immediately after check?
  21. Any Global variable exist? If so, is there possibility to be called from more than one pre-emptive tasks or ISR? If so, are they protected with mutual exclusion? Will this function be called from more than one context? If so, is this re-entrant?
  22. Optimized (Rearrange fields of structure so that all variables are placed in boundary so that the sizeof the structure can be reduced, etc,.)?

Please give your comments, if you have any specific doubt in the above. See you in the next post, "Design Guidelines that matters Embedded Engineers"

3 comments:

Anonymous said...

If structure is released, all member variables are released before struct released?

If calling thread, the paased parameter to the thread is dynamically allocated?

Whether all error conditions have log? Logs have all key information for debug?

If there are multiple threads, whether timing synchronization is performed?

Unknown said...

In any calculation, is there any chance of Overflow?

Anonymous said...

If any var++ has been done, whether limit is checked? If no, this will overwrite other variables.