virtual.c: Fix compiler warnings related to interface name

[clamattia]

Fix compiler warning by adjusting the number of chars copied to the
destination. Compiler does not like if the destination size of the
strncpy-operation is the same as the number of characters written. Even
though it is not a bug in this case. Only copying size-1 characters fixes
the warning and exhibits the same behavior.

[clamattia]

what is /** @endcond */ ? Should it stay?

[rlubos]

It's termination for the /** @cond INTERNAL_HIDDEN */ above, it keeps part of the header excluded from doxygen processing.

[clamattia]

Thanks, added it back.

[clamattia]

Added doxygen endcond back

[jukkar]

The commit log topic should be
net: virtual: ...

But before fully accepting this, I would like to know why this code is a problem.

[jukkar]

The whole point of the original code was that we do not need to add this ifdef here.
What is the problem with the compiler in this case, why it is complaining about 0 size array?
Note that if CONFIG_NET_L2_VIRTUAL is not set, this struct is never used anywhere so this sounds like a false positive.

[clamattia]

Agree, that it was probably a false positive.

Removing the 0-length array was my addition. Because, as far as I know zero-length arrays are not portable. If it is not the case, I will revert, but then this code ctx->name[ARRAY_SIZE(ctx->name) - 1] = '\0'; looks dangerous (I know it is only compiled in if ctx->name > 0 but still. In that case I will add a static assertion instead.

Both patches, make it clearer IMO, because no context knowledge is needed.
For example

	strncpy(ctx->name, name, ARRAY_SIZE(ctx->name));
        ctx->name[ARRAY_SIZE(ctx->name) - 1] = '\0';

Is clear without additional context.

In the old code. The reader hat to know that. In some circumstances CONFIG_NET_L2_VIRTUAL_MAX_NAME_LEN was the length of the ctx->name and in others this function would not get compiled it.

Adjusted the commit message.

[clamattia]

@jukkar I could use 1 instead of 0. Would you prefer this?

[jukkar]

Perhaps we should do [] instead which is portable, see #81895

[clamattia]

#if defined(CONFIG_NET_L2_VIRTUAL)
#define VIRTUAL_MAX_NAME_LEN CONFIG_NET_L2_VIRTUAL_MAX_NAME_LEN
#else
/* Leave empty so it creates a flexible array member at the end */
#define VIRTUAL_MAX_NAME_LEN
#endif

Flexible array members are not my favorite. But I could live with it. Note for example MISRA disallows it. I believe it is intended only as last element of structs that are intended for dynamic allocation.

[jukkar]

But here the flex array is only used to silence the compiler, we are not using the struct in this case anywhere.

[clamattia]

Ok, will try.

[clamattia]

Found the true source of the warning and adjusted the code and the description of this PR.
Please have another look @jukkar

[clamattia]

(Also ajusted commit message)

[jukkar]

Thanks, this looks good now.

[clamattia]

Thanks, this looks good now.

Thank you for the quick feedback 👍