[RFC] limiting visibility of internal structures #8100
Description
PROBLEM:
there are many structures in SOF that should be modified by dedicated functions only
BUT, unless the structure is protected somehow, it is extremely easy for anybody to access them directly. A developer may not even know that there are special API functions for reading/writing values,
Review is fine, but sooner or later somebody will, even accidently, violate the rule and make problems with further code flexibility
In C++ the issue is solved by compiler and public: / protected: / private: members of a class
In C, however, there's no perfect way to do it
The only way to make some of members private is to limit their range to single .c file (put a struct definition in .c) and export only proper API functions.
Unfortunately such API functions cannot be inline and therefore have significant performance impact
The purpose of this RFC is a brainstorm - how to do it to have a pretty good protection without loosing performance?
In example, FREE Rtos is doing this in a very hard way, having always 2 versions of structures: internal (in .c file) and exported (in .h), obfuscating all struct members names to meaningless dummy1, dummy2, etc.
i,e, exported:
typedef struct xSTATIC_TIMER
{
void * pvDummy1;
StaticListItem_t xDummy2;
TickType_t xDummy3;
void * pvDummy5;
TaskFunction_t pvDummy6;
#if ( configUSE_TRACE_FACILITY == 1 )
UBaseType_t uxDummy7;
#endif
uint8_t ucDummy8;
} StaticTimer_t;
internal
typedef struct tmrTimerControl /* The old naming convention is used to prevent breaking kernel aware debuggers. */
{
const char * pcTimerName; /**< Text name. This is not used by the kernel, it is included simply to make debugging easier. */ /*lint !e971 Unqualified char types are allowed for strings and single characters only. */
ListItem_t xTimerListItem; /**< Standard linked list item as used by all kernel features for event management. */
TickType_t xTimerPeriodInTicks; /**< How quickly and often the timer expires. */
void * pvTimerID; /**< An ID to identify the timer. This allows the timer to be identified when the same callback is used for multiple timers. */
portTIMER_CALLBACK_ATTRIBUTE TimerCallbackFunction_t pxCallbackFunction; /**< The function that will be called when the timer expires. */
#if ( configUSE_TRACE_FACILITY == 1 )
UBaseType_t uxTimerNumber; /**< An ID assigned by trace tools such as FreeRTOS+Trace */
#endif
uint8_t ucStatus; /**< Holds bits to say if the timer was statically allocated or not, and if it is active or not. */
} xTIMER;
This way is much to complicated in my opinion (yet really efficient)
MY PROPOSITION
if a member of a structure needs to be like C++ private, add a prefix, like PRIVATE or READONLY
i.e.
/** internals of sink API. NOT TO BE MODIFIED OUTSIDE OF sink_api_helper.h */
struct sof_sink {
const struct sink_ops *_READONLY_ops; /** operations interface */
size_t _PRIVATE_requested_write_frag_size; /** keeps number of bytes requested by get_buffer() */
size_t _PRIVATE_num_of_bytes_processed; /** processed bytes counter */
size_t _PRIVATE_obs; /** output buffer size as declared in module bind IPC */
struct sof_audio_stream_params *_PRIVATE_audio_stream_params; /** pointer to audio params */
};
I know it is not perfect nor pretty, yet if a developer sees a prefix, it will think twice before accessing the field directly
Any other ideas?