Re: [SC-L] Conditional Compile statements-- coding standards, and code review
When my organization develops code in C++, we generally ban use of #ifdef and #if defined(X), but we otherwise allow use of #if. The reason is that if you mis-spell the identifier that follows #ifdef, the compiler can't warn you. For example, if you write #ifdef FRDE ... #endif when you meant #ifdef FRED, the compiler doesn't warn you, and the conditional may not be interpreted as was intended. Best regards David Crocker, Escher Technologies Ltd. http://www.eschertech.com -Original Message- From: sc-l-boun...@securecoding.org [mailto:sc-l-boun...@securecoding.org] On Behalf Of Bennett, Jason Sent: 16 February 2009 09:46 To: 'sc-l@securecoding.org' Subject: Re: [SC-L] Conditional Compile statements-- coding standards,and code review 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 postmas...@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. ___ Secure Coding mailing list (SC-L) SC-L@securecoding.org List information, subscriptions, etc - http://krvw.com/mailman/listinfo/sc-l List charter available at - http://www.securecoding.org/list/charter.php SC-L is hosted and moderated by KRvW Associates, LLC (http://www.KRvW.com) as a free, non-commercial service to the software security community. ___ ___ Secure Coding mailing list (SC-L) SC-L@securecoding.org List information, subscriptions, etc - http://krvw.com/mailman/listinfo/sc-l List charter available at - http
Re: [SC-L] Conditional Compile statements-- coding standards, and code review
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 postmas...@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. ___ Secure Coding mailing list (SC-L) SC-L@securecoding.org List information, subscriptions, etc - http://krvw.com/mailman/listinfo/sc-l List charter available at - http://www.securecoding.org/list/charter.php SC-L is hosted and moderated by KRvW Associates, LLC (http://www.KRvW.com) as a free, non-commercial service to the software security community. ___
Re: [SC-L] Conditional Compile statements-- coding standards, and code review
Sean, I think you would want to provide this guarantee through some sort of static assertion. For example, if you want to ensure that text controlled by FRED is not included in a release build, you could include an #error preprocessor directive as part of the controlled text that will generate an error message for a release build: #ifdef FRED # define MACRO(x) (x + 5) # ifdef NDEBUG # error FRED defined in release build # endif #endif The idea here is that NDEBUG would be defined for a release build. If FRED and NDEBUG were defined in the same build it would result in a fatal compile-time diagnostic. I'm not sure if there is a more elegant or widely deployed solution to this problem. rCs -Original Message- From: sc-l-boun...@securecoding.org [mailto:sc-l-boun...@securecoding.org] On Behalf Of smurray1 Sent: Friday, February 13, 2009 10:49 AM To: sc-l@securecoding.org Subject: [SC-L] Conditional Compile statements-- coding standards, and code review I am reviewing a QA team's procedures for code review. I have an issue with conditional compile statements (#ifdef in the C world). My issue is that it is very difficult to have complete confidence that a piece of code inside the condition (the controlled text) does indeed not get compiled and included in the final executable. The coding standards used by the organization are fairly rigorous, but there is no mention of prohibiting (or of even limiting) the used of conditional compile statements. They are typically used for debug purposes-- that is, debug messages that get generated when the code is compiled for debugging and then are omitted in the production builds. This is probably more of a correct code issue than a security issue, but there are most definitely security implications. I am curious to hear people's thoughts on this. Do most organizations prohibit (or at least limit) conditional compile statements? If not, how is the controlled text inside conditional compile statements handled by code reviewers? The QA procedures I am reviewing basically ignore them, since They won't be in the production build, but I am very uncomfortable with that. There are many ways in C to define the macro that controls the conditional compile (with #define statements, with compiler flags, etc). It just seems very hard to verify that the ifdefs will work as planned in the final compile. Thanks!! Sean T Murray ___ Secure Coding mailing list (SC-L) SC-L@securecoding.org List information, subscriptions, etc - http://krvw.com/mailman/listinfo/sc-l List charter available at - http://www.securecoding.org/list/charter.php SC-L is hosted and moderated by KRvW Associates, LLC (http://www.KRvW.com) as a free, non-commercial service to the software security community. ___ ___ Secure Coding mailing list (SC-L) SC-L@securecoding.org List information, subscriptions, etc - http://krvw.com/mailman/listinfo/sc-l List charter available at - http://www.securecoding.org/list/charter.php SC-L is hosted and moderated by KRvW Associates, LLC (http://www.KRvW.com) as a free, non-commercial service to the software security community. ___
