On 02/13/2013 10:44 AM, Olivier Martin wrote:
> Instead of using:
> #if !defined(MDEPKG_NDEBUG)
> (...)
> #endif
>
> It would be better to use:
> DEBUG_CODE_BEGIN();
> (...)
> DEBUG_CODE_END();
>
> The reason is you can be in debug build and disable debug code (and/or
> disable other things such as ASSERT). See definition of PcdDebugPropertyMask
> for the full list of debug properties.
These macros insert code which would place the variable definition
inside an "if" statement. This usage violates the Coding Standard on
page 30:
"The data declarations must be the first code in a module."
"Data declarations appearing anywhere but at the beginning of the
module are illegal."
> The EDK2 coding convention
> (http://sourceforge.net/projects/edk2/files/General%20Documentation/EDK%20II
> %20C%20Coding%20Standards%20Specification.pdf/download) says we should not
> write this kind of variable declaration:
> EFI_STATUS Status = EFI_INVALID_PARAMETER;
>
> See page 30: "Initializing a variable as part of its declaration is
> illegal."
This restriction is violated in almost every case in the files I
patched. I merely coded my changes to blend in with the surrounding
code, as any good maintenance programmer would do.
The coding standard's requirement is, in this case, contrary to commonly
accepted coding practice in the industry for the last decade. It really
should be removed from the DRAFT standard.
The mandatory space between the function name and the opening paren of
its argument list needs to go away too: the vast majority of the
programmers I've spoken with read that as missing a binary operator
between a variable and a parenthized expression. They have to re-read
the code several times before it becomes apparent that this is a
function call and not a coding error. The existence of documented
exceptions (macro definitions, etc.) support the argument that it's a
bad practice.
Feel free to change the patch as you see fit. I never expected to
publish these changes; they were intended only to help me figure out why
I couldn't build a standard target in my development environment. Ryan
asked for copies so I provided them.
-Reece
_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
