treewide: initialize several stack-allocated, but uninitialized timer structs

[miri64]

Contribution description

While digging around for my TinyDTLS issue reported in #17849, I did some valgrind probes with gcoap_dtls and noticed some missing initializations of timers. This PR fixes those.

Testing procedure

Compile examples/gcoap_dtls with valgrind support (for native) and start a shell with valgrind support:

sudo dist/tools/tapsetup/tapsetup
make -C examples/gcoap_dtls -j all-valgrind
make -C examples/gcoap_dtls term-valgrind

open another shell

PORT=tap1 make -C examples/gcoap_dtls term-valgrind

and send a coap request using the coap command. With this PR, valgrind will remain mostly silent, while in current master it will complain about several "Conditional jump or move depends on uninitialised value(s)"

e.g.:

==76316==    at 0x806F620: ztimer64_remove (sys/ztimer64/ztimer64.c:64)
==76316==    by 0x805C8D2: xtimer_remove (sys/include/ztimer64/xtimer_compat.h:150)
==76316==    by 0x805C8D2: gnrc_sock_recv (./gnrc_sock.c:143)
==76316==    by 0x805CFAE: sock_udp_recv_buf_aux (sys/net/gnrc/sock/udp/gnrc_sock_udp.c:237)
==76316==    by 0x806EDC0: sock_udp_recv_buf (/home/mlenders/Repositories/RIOT-OS/RIOT/sys/include/net/sock/udp.h:605)
==76316==    by 0x806EDC0: _udp_cb (./sock_dtls.c:872)
==76316==    by 0x806696B: _event_handler (sys/net/sock/async/event/sock_async_event.c:33)
==76316==    by 0x80501F8: event_loop_multi (sys/include/event.h:409)
==76316==    by 0x80501F8: event_loop (sys/include/event.h:431)
==76316==    by 0x80501F8: _event_loop (./gcoap.c:183)
==76316==    by 0x41BE788: makecontext (in /usr/lib32/libc.so.6)
==76316==  Uninitialised value was created by a stack allocation
==76316==    at 0x805C7E4: gnrc_sock_recv (sys/net/gnrc/sock/gnrc_sock.c:97)

Issues/PRs references

None

[kaspar030]

ACK.

[kaspar030]

maybe it should be ztimer_t foo = {0}, not sure, probably slower but less likely to break.

[miri64]

maybe it should be ztimer_t foo = {0}, not sure, probably slower but less likely to break.

I seem to remember there were some problems with = {0} in general, so that's mostly why I chose some well known members. 🤷‍♀️

[miri64]

maybe it should be ztimer_t foo = {0}, not sure, probably slower but less likely to break.

It all non-mentioned members will be set to zero anyway, but .next is the only one kept uninitialized (callback and args are usually set later on in the code)

[kfessel]

If the initialsation is done with = {.callback = _callback_put} the rest of the struct should be initialised (to zero) as well.

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf (p.:142 pdf.:160 l:28 there is an example how to handel partial initialisation)

[miri64]

If the initialsation is done with = {.callback = _callback_put} the rest of the struct should be initialised (to zero) as well.

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf (p.:142 pdf.:160 l:28 there is an example how to handel partial initialisation)

That should be the case for the way I picked it as well. That's why I picked a member where I know it should be 0 in any case.

[kfessel]

Suggested change

	ztimer_t timeout_timer = { .base = { .next = NULL } };
	ztimer_t timeout_timer = {.callback = _callback_put};

[miri64]

The callback is only needed for when the if is set below. So I would rather not initialize something that is not needed and go for the NULL initializer as picked.

[kfessel]

how about:

Suggested change

	ztimer_t timeout_timer = { .base = { .next = NULL } };
	ztimer_t timeout_timer = { .callback = NULL };

or

Suggested change

	ztimer_t timeout_timer = { .base = { .next = NULL } };
	ztimer_t timeout_timer = { 0 };

not naming parts of the ztimer_t struct that a user of ztimer should not have to deal with.

[kaspar030]

yeah, maybe be a good example.

https://github.com/kaspar030/RIOT/blob/ef4b453d4dde0122dee7fcc511f474d5a09f0750/sys/include/ztimer.h#L76-L82

[miri64]

Bump github actions.

[kaspar030]

I seem to remember there were some problems with = {0} in general, so that's mostly why I chose some well known members. woman_shrugging

problems?

[kfessel]

I seem to remember there were some problems with = {0} in general, so that's mostly why I chose some well known members. woman_shrugging

problems?

i think the problem is generated with c++ where {0} can only be used types that can be initialized by a scalar scalars and {} is the default initializer for struct and co.
but i could not generate any error for ztimer_t initializing it using {0}.

[miri64]

i think the problem is generated with c++ where {0} can only be used types that can be initialized by a scalar scalars and {} is the default initializer for struct and co.
but i could not generate any error for ztimer_t initializing it using {0}.

IIRC the problem was also related to compiling on OSX using XCode, but tbh I don't know anymore. If there is no problem, I can adjust my style :-)

[chrysn]

Just saw this now, and not objecting, but apart from valgrinding RIOT, might these cases be spottable more easily if we (by setting a flag, or merely by stating it needs to be done this way, even if the checks are only in with DEVELHELP) made ztimer complain loudly (as in assert) if a timer is added that is neither zerod where it matters nor on the clock it is being added to?