Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
miri64 merged 2 commits into from
Mar 28, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5aefb8d
gnrc_sock: initialized un-initialized timer objects
miri64 Mar 24, 2022
a81c5a5
ztimer: initialized un-initialized timer objects
miri64 Mar 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sys/net/gnrc/sock/gnrc_sock.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ ssize_t gnrc_sock_recv(gnrc_sock_reg_t *reg, gnrc_pktsnip_t **pkt_out,
return -EINVAL;
}
#if IS_USED(MODULE_ZTIMER_USEC)
ztimer_t timeout_timer;
ztimer_t timeout_timer = { .base = { .next = NULL } };

@kfessel kfessel Mar 24, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 miri64 Mar 28, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 kfessel Mar 24, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@miri64 miri64 Mar 28, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 kfessel Mar 28, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 kaspar030 Mar 28, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, maybe be a good example.

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

if ((timeout != SOCK_NO_TIMEOUT) && (timeout != 0)) {
timeout_timer.callback = _callback_put;
timeout_timer.arg = reg;
ztimer_set(ZTIMER_USEC, &timeout_timer, timeout);
}
#elif IS_USED(MODULE_XTIMER)
xtimer_t timeout_timer;
xtimer_t timeout_timer = { .callback = NULL };

/* xtimer_spin would make this never receive anything.
* Avoid that by setting the minimal not spinning timeout. */
Expand Down
2 changes: 1 addition & 1 deletion sys/ztimer/util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ int ztimer_msg_receive_timeout(ztimer_clock_t *clock, msg_t *msg,
return 1;
}

ztimer_t t;
ztimer_t t = { .base = { .next = NULL } };
msg_t m = { .type = MSG_ZTIMER, .content.ptr = &m };

ztimer_set_msg(clock, &t, timeout, &m, thread_getpid());
Expand Down