Secure Coding mailing list archives

Conditional Compile statements-- coding standards, and code review


From: Jason.Bennett at thales-esecurity.com (Bennett, Jason)
Date: Mon, 16 Feb 2009 09:46:05 -0000

Robert/Sean,

It's a good question and one that I've never seen a really good answer to!
Robert your option certain works but I feel that it somewhat prone to error
if deployed on a large source base. So for example if a developer actually
uses:

#ifdef FRED
#  define MACRO(x) (x + 5)
#endif

... then it's quite possible that this is missed by the review team and
there is of course no guarantee that all code is reviewed manual. There is
also the problem that there may be more than a single target release build
for different variants i.e. it's not just a binary choice of release or
debug versions.

To make a more 'fool proof' mechanism I believe that it's better to have a
more controlled use of which pre-processor directives are allowed for
conditional compilation and ensure their use is consistent -- this is
particular true of debug information which I believe causes the most
problems. Following this approach would allow you to perform automatic
searches for directives that are not on a defined white list. A word of
warning this isn't as easy as it seems once you start getting statements of
the following type -- this just re-enforces the problem of conditional
compilation:

#if defined c1 && !(defined c2 || defined c3)
 ...
#elif defined C4
 ...
#endif

What would be really nice is to have an automatic tool that can check that
for say build target A you can only have I, J and K defined but for not L
and M -- using 3rd party code which is often designed to be ported to
multiple targets sorting out what is actually used is not easy at all!

Use should also looked at carefully to ensure that conditional compilation
is only used where 'required'. So as an example do you really want all those
call traces and information output used during development left in the code?

In conclusion I believe that you should aim for as much automation as
possible and also taking the problem out of the developer's hands. It's much
easier to ensure that you've done something right once in your build system
than expect every developer to do it right every time -- in my experience
developers are happy to change what is in their 'local domain' but think
about things a bit more carefully if they are making a change the can affect
the entire development.

Obviously these are just some ideas and I'm sure that there or other equally
good solutions and as with all these things it does depend on what level of
assurance you want otherwise you get the answer of don't allow conditional
compilation! 
Consider the environment before printing this mail.
"Thales e-Security Limited is incorporated in England and Wales with company
registration number 2518805. Its registered office is located at 2 Dashwood
Lang Road, The Bourne Business Park, Addlestone, Nr. Weybridge, Surrey KT15
2NX.
The information contained in this e-mail is confidential. It may also be
privileged. It is only intended for the stated addressee(s) and access to it
by any other person is unauthorised. If you are not an addressee or the
intended addressee, you must not disclose, copy, circulate or in any other
way use or rely on the information contained in this e-mail. Such
unauthorised use may be unlawful. If you have received this e-mail in error
please delete it (and all copies) from your system, please also inform us
immediately on +44 (0)1844 201800 or email postmaster at thales-esecurity.com.
Commercial matters detailed or referred to in this e-mail are subject to a
written contract signed for and on behalf of Thales e-Security Limited". 


Current thread: