[cujomalainey]

The init functions are declared in a way that upsets checkpatch, this fixes those errors.

Fixes #1166

[ranj063]

LGTM. Thanks @cujomalainey

[lgirdwood]

@abonislawski any idea why the UT's would fail here ? I'm adding you as this is a valid fix for the self registering components but @cujomalainey cannot see the UT results as it's an internal link atm. Can you help resolve this. Thanks.

[abonislawski]

@lgirdwood yes, tests fails because init functions are no longer visible. For example: test/cmocka/src/audio/mixer/mixer_test.c:193

We cannot use static function with the current test implementation

[jajanusz]

@lgirdwood @abonislawski @cujomalainey
I'll add fixing commit to this PR, wait for it

[jajanusz]

After some thinking it's going a bit wrong way.

It can be fixed with something like

/** \name Declare component macro
 *  \brief Usage at the end of comp file: DECLARE_COMPONENT(sys_comp_*_init);
 *  @{
 */
#ifdef CHECK
#define DECLARE_COMPONENT(init) \
	void ut_alias_ ## init(void) __attribute__((weak, alias(#init)))
#else
#define DECLARE_COMPONENT(init) __attribute__((__used__)) \
	__attribute__((section(".comp_init"))) static void(*f)(void) = init
#endif

And then we will use ut_alias_some_comp_init() in unit tests, it's going to be exported just for unit tests. But I don't like it at all.

First of all we should fix checkpatch to treat DECLARE_COMPONENT just like EXPORT_SYMBOL.
Then it depends if we want comp_inits to be visible. I like them being static, but if we want them to be static and still testable, I prefer it to be done with something like:

UT_STATIC void some_comp_init(){...}

Where UT_STATIC is:

#ifdef CHECK
#define UT_STATIC
#else
#define UT_STATIC static
#endif

Why is it better than aliases? - Sooner or later we will need some way to test other functions that may be static and they will be most likely done like this instead of aliases, so it's better to have 1 mechanism for exporting static functions for UT than 2, just for simplicity.

[abonislawski]

@jajanusz second option is much better and ready for general use in the future as you described.

But in general there is no clear answer in my opinion. For example static functions are restricted to use only in this file but in fact we will use it in component.c so actually they shouldnt be static to inform about that.
On the other hand static can limit the usage only to component.c in this case

[jajanusz]

@cujomalainey
You can merge this: https://github.com/cujomalainey/sof/pull/1
It will automatically update this PR and rerun CI checks once you merge it

[abonislawski]

Looks fine