sys: util: Prevent ARRAY_SIZE being defined if already defined

[nordicjm]

Prevents ARRAY_SIZE being defined by util.h if it is already defined,
prevents possible future include file ordering issues that have
plagued CI tests in the past.

[andyross]

Where is the other definition? If it's in the Zephyr tree, we should fix that. If it's not, then we have an API collision between modules and we need to think about a better solution (e.g. fix it in the foreign code, or rename it to "SYS_ARRAY_SIZE()" or whatever to be part of a known Zephyr namespace).

[nordicjm]

@andyross There is the same macro in zcbor, the zcbor code only defines it if it is not already defined

[de-nordic]

@andyross There is the same macro in zcbor, the zcbor code only defines it if it is not already defined

@nordicjm I do not think that Zephyr provided macro definition should be conditional, as it would allow to override macro behaviour in out of tree code - making it behave differently in Zephyr and external code.
Maybe not in this situation, but other macros included via util.h may depend on the macro and their behaviour may also change.
When using Zephyr macros you relay on specific behaviour that is documented for Zephyr, if you want other behaviour then you have to explicitly undef the Zephyr macro and redefine it.

I am -1 on this change. Sorry.

If it's not, then we have an API collision between modules and we need to think about a better solution (e.g. fix it in the foreign code, or rename it to "SYS_ARRAY_SIZE()" or whatever to be part of a known Zephyr namespace).

@andyross It might be good idea to make some of Zephyr macros, by for example SYS_ prefixing, more unique to decrease chance of collision. I am quite sure there has been discussion on that some time ago... I may be wrong.

[stephanosio]

While this may be a bit beyond the scope of this PR, as @andyross noted, it would be preferable to rename these macros so as to minimise the chance of name conflicts.

[andyross]

FWIW: for this specific macro, there is a long history of its use in Linux, where it's defined globally and used pervasively. I think that at least makes a reasonable precedent that it's a standard "OS kernel" API and thus something we should feel free to define ourselves.

I don't know zcbor at all, but if it's colliding here maybe the solution is to put its definition inside an #ifndef __ZEPHYR__?

[nordicjm]

FWIW: for this specific macro, there is a long history of its use in Linux, where it's defined globally and used pervasively. I think that at least makes a reasonable precedent that it's a standard "OS kernel" API and thus something we should feel free to define ourselves.

I don't know zcbor at all, but if it's colliding here maybe the solution is to put its definition inside an #ifndef __ZEPHYR__?

zcbor has #ifndef protection around it, it's when you have includes including includes including includes, etc. whereby util.h gets included in one specific source file after the zcbor header has been included

[andyross]

Right, what I'm saying is that zcbor should have something like this instead, in whatever header it's defining ARRAY_SIZE():

#ifdef __ZEPHYR__
#include <zephyr/sys/util.h>
#else
#define ARRAY_SIZE(a) ...
#endif

[de-nordic]

Right, what I'm saying is that zcbor should have something like this instead, in whatever header it's defining ARRAY_SIZE():

#ifdef __ZEPHYR__
#include <zephyr/sys/util.h>
#else
#define ARRAY_SIZE(a) ...
#endif

You can always do something like:

#ifndef  ZCBOR_USE_EXTERNAL_ARRAY_SIZE_DEFINITION //bad name
#undef ARRAY_SIZE(a) ...
#define ARRAY_SIZE(a) ...
#endif

where ZCBOR_USE_EXTERNAL_ARRAY_SIZE_DEFINITION would be provided via, for example, -D.

It then forces explicit decision whether the ARRAY_SIZE is taken from zcbor or something zcbor can not control directly.