17. Macro Use Policy
The problem:
A piece of code like this (e.g. in a .H file):
#if (GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES > PMPTASK__MIN_SHARED_BUFFER_SIZE)
#define PMPTASK__SHARED_BUFFER_SIZE GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES
#else
#define PMPTASK__SHARED_BUFFER_SIZE PMPTASK__MIN_SHARED_BUFFER_SIZE
#endif
followed by code like this (e.g. in the paired .C file):
uint8_t gui8aSharedBuffer[PMPTASK__SHARED_BUFFER_SIZE];
is subtlely EXTREMELY dangerous. Why? Because
GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES
is defined in another file! And when it is
not defined at all (or when the “other” file is not included with an #include), the
preprocessor — get this —
silently evaluates the #if as FALSE
instead of reporting GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES
as undefined. So the
buffer will have PMPTASK__MIN_SHARED_BUFFER_SIZE (1024)
from some modules, and
GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES (2048)
from other modules that HAPPENED to
include the file where GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES
is defined before this
.H file is included.... This actually happened in the Dash-2 project and it took 8
hours to trace down. What this caused: the linker, literally, overlapped 2 arrays:
gui8aSharedBuffer[2048]
and
sui8aFtlDriveBuffer[8624]
where most of the 2nd half of the former was positioned overlapping the latter! This is understandable since some object files saw the array as 1024 bytes and others saw it as 2048, so it is just a roll of the dice as to which one resulted, since the linker would have to trust the first object file that indicated its size.
Thankfully, this caused a consistent CPU BUS ERROR exception when a NULL pointer was being used by the SafeFTL library during copying files! This COULD have gone undetected for a long time.... Or it could even have shown up in the field, or at HQ after software delivery! Alternatively, if it hadn’t caused a detectable problem, it could have easily made it into the field.... And tracing it down could have cost weeks or months, not to mention unhappy customers!
So the new firm policy is: if there is EVER a variable array size like the above, then the code MUST be set up like this (with a sanity-check above it), so that it is guaranteed to have all the values required when the preprocessor passes this code that compilation stops with an error message if any required value is missing:
#if !defined(GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES) || !defined(PMPTASK__MIN_SHARED_BUFFER_SIZE)
#error GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES is mandatory here, lest the linker fatally sees this array as 1024 from some modules and 2048 from others!!!! AAAAAAhhhhh!!!!!
#elif (GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES > PMPTASK__MIN_SHARED_BUFFER_SIZE)
#define PMPTASK__SHARED_BUFFER_SIZE GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES
#else
#define PMPTASK__SHARED_BUFFER_SIZE PMPTASK__MIN_SHARED_BUFFER_SIZE
#endif
With this in place, then the .H file received the vital, but missing #include
:
#include "FileSystem.h" /* For GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES definition. */
Alternatively, it can be done like this, which can be neater and easier to read:
/*-------------------------------------------------------------------------
* Sanity Checks
*-------------------------------------------------------------------------*/
#if !defined(GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES)
#error GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES must be defined in _UITask.h and included above, and be in a reasonable range.
#elif (GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES >= 512)
/* Reasonable Range: OK */
#else
#error GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES must be defined in _UITask.h and included above, and be in a reasonable range.
#endif
#if !defined(PMPTASK__MIN_SHARED_BUFFER_SIZE)
#error PMPTASK__MIN_SHARED_BUFFER_SIZE must be defined in PMP.h and included above, and be in a reasonable range.
#elif (PMPTASK__MIN_SHARED_BUFFER_SIZE >= 512)
/* Reasonable Range: OK */
#else
#error PMPTASK__MIN_SHARED_BUFFER_SIZE must be defined in PMP.h and included above, and be in a reasonable range.
#endif
...(later)...
#if (GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES > PMPTASK__MIN_SHARED_BUFFER_SIZE)
#define PMPTASK__SHARED_BUFFER_SIZE GFX__MIN_FILE_BUFFER_SIZE_IN_BYTES
#else
#define PMPTASK__SHARED_BUFFER_SIZE PMPTASK__MIN_SHARED_BUFFER_SIZE
#endif
...(later)...
uint8_t gui8aSharedBuffer[PMPTASK__SHARED_BUFFER_SIZE];
This is safe and will keep you out of such trouble.