llvm: Fix unused function warning

[Teufelchen1]

Related to #18851

When building example/gnrc_lorawan using LLVM two unused function error are raised. Those are fixed by only include said functions when they are needed.

Additionally, LLVM complains about lorawan struct being not packed correctly. I believe this is because GCC applies the given attribute(packed) to all elements of a given struct including the anonymous struct and unions if present. Clang on the other does not seem to do that. The fix works by adding the packed attribute to the anonymous union as well.

[kfessel]

Suggested change

	union __attribute__((packed)) {
	uint32_t u8_pad;
	le_uint16_t conf_fcnt;
	};
	uint16_t u8_pad;
	le_uint16_t conf_fcnt;

i am not sure why that pad is named u8_pad maybe you are able to tell?

[benpicco]

Shouldn't that be

le_uint16_t conf_fcnt;
uint16_t u8_pad;

[kfessel]

you are right

Suggested change

	union __attribute__((packed)) {
	uint32_t u8_pad;
	le_uint16_t conf_fcnt;
	};
	le_uint16_t conf_fcnt;
	uint16_t u16_pad;

[kfessel]

diff --git a/sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_crypto.c b/sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_crypto.c
index 18fc6e29b9..6bcddfde2d 100644
--- a/sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_crypto.c
+++ b/sys/net/gnrc/link_layer/lorawan/gnrc_lorawan_crypto.c
@@ -235,11 +235,8 @@ void gnrc_lorawan_encrypt_payload(iolist_t *iolist, const le_uint32_t *dev_addr,
                                   uint32_t fcnt, uint8_t dir,
                                   const uint8_t *appskey)
 {
-    uint8_t s_block[16];
-    uint8_t a_block[16];
-
-    memset(s_block, 0, sizeof(s_block));
-    memset(a_block, 0, sizeof(a_block));
+    uint8_t s_block[16]={0};
+    uint8_t a_block[16]={0};
 
     lorawan_block0_t *block = (lorawan_block0_t *)a_block;
 
@@ -247,14 +244,11 @@ void gnrc_lorawan_encrypt_payload(iolist_t *iolist, const le_uint32_t *dev_addr,
 
     block->fb = CRYPT_B0_START;
 
-    block->u8_pad = 0;
     block->dir = dir & DIR_MASK;
 
     block->dev_addr = *dev_addr;
     block->fcnt = byteorder_htoll(fcnt);
 
-    block->u32_pad = 0;
-
     int c = 0;
 
     for (iolist_t *io = iolist; io != NULL; io = io->iol_next) {

[Teufelchen1]

Rebased my PR. This was fixed here: 4709bbb in PR #19634

Why was this discussed here but not there? I suspect it slipped through, as the PR had well over 1500 edits? We really should aim for small PRs when feasible :)

[kfessel]

either use attribute unused or move the #ifdef up from line 244 (not do the same #ifdef twice)

[kfessel]

#18884

[Teufelchen1]

I added a fixup that moves the #ifdef. I think attribute(unused) is wrong here. This is not three steps into linking or hidden behind 100 layers of dependency modelling. That function is exactly used three times and only when that option is enabled. An attribute does not convey that meaning/context but the ifdef does.

[kfessel]

this may better be two PR since the changes are in very different parts of RIOT and not connected but by this PR and the issue

[maribu]

Suggested change

	#if IS_ACTIVE(CPU_HAS_BACKUP_RAM)
	__attribute__((unused))

is IMO much better than using the preprocessor, as we get better LSP suggestions, better static analysis results and wide compile test coverage when not hiding functions from the compiler.

[Teufelchen1]

Hehe, you did it yourself in f4c5cf1

[riot-ci]

Murdock results

✔️ PASSED

d19182c llvm: cpu/stm32: Fix unused function warning

Success	Failures	Total	Runtime
10008	0	10009	09m:10s

Artifacts

[benpicco]

Those are both very minor changes, I don't think we need two PRs for that.

[benpicco]

Suggested change

	#if defined(STM32_OPTION_BYTES)
	MAYBE_UNUSED

[Teufelchen1]

Saw this comment too late, see #18870 (comment)

[benpicco]

Please squash

[maribu]

Looks like something has gone wrong when squashing scratch that, I misread

[Teufelchen1]

Switched to using MAYBE_UNUSED after maribu asked in private politely.