gnrc_sock: set remote network interface implicitly#8470
gnrc_sock: set remote network interface implicitly#8470bergzand merged 2 commits intoRIOT-OS:masterfrom
Conversation
miri64
added
Type: enhancement
| static inline void gnrc_ep_set(sock_ip_ep_t *out, const sock_ip_ep_t *in, | ||
| size_t in_size) | ||
| { | ||
| memcpy(out, in, in_size); |
There was a problem hiding this comment.
Is there a reason as not to fix the size to sizeof(sock_ip_ep_t) here (and remove the extra argument with it)?
There was a problem hiding this comment.
Yes, in https://github.com/RIOT-OS/RIOT/pull/8470/files#diff-71ddba2942574ad9a8d73c8de31b2230R126 this is called with sizeof(sock_udp_ep_t) ;-).
There was a problem hiding this comment.
Never mind then, It all looked like the same sock_something_ep_t from here :)
| gnrc_netif_t *netif = gnrc_netif_iter(NULL); | ||
|
|
||
| if (netif != NULL) { | ||
| rem->netif = netif->pid; |
There was a problem hiding this comment.
If I read the code correctly it is possible that this changes the const sock_udp_ep_t *remote argument here (first assigning remote to rem and then changing rem here).
Besides changing the argument to non const, I don't really have a solution, but I think it is a bit ugly to modify something that is passed as const
There was a problem hiding this comment.
Mhhh change it to non-const is of the table, but you are right! This is bad. I try to come up with a better solution.
|
Tested, works. Please squash. |
|
Tested. Works like a charm. |
miri64
force-pushed
the
gnrc_sock/enh/set-remote-netif-implicitly
branch
from
936b3b8 to
98e45c3
Compare
|
Squashed. Sorry for the delay. |
When there is only one interface we are simplifying a lot for the users if the interface is set implicitly.
miri64
added
the
CI: ready for build
|
Just to be sure, Travis/cppcheck complaints look not completely valid to me |
|
Yes, but I think it doesn't hurt to copy the cpp-check suppression from ll246ff ;-) |
|
Done |
|
I think the suppression needs to be above line 255, travis is still complaining |
miri64
force-pushed
the
gnrc_sock/enh/set-remote-netif-implicitly
branch
from
846dcf9 to
527b850
Compare
Arghs that comes from editing on master first and then changing to the branch ^^". Fixed and amended immediately. |
miri64
force-pushed
the
gnrc_sock/enh/set-remote-netif-implicitly
branch
from
527b850 to
df8d0d9
Compare
|
And now the cppcheck from murdock started to complain. Looks like the same cppcheck exception should be added to the comment at l143 of |
miri64
force-pushed
the
gnrc_sock/enh/set-remote-netif-implicitly
branch
from
df8d0d9 to
15d405f
Compare
|
Fixed and amended immediately. |
3 participants
Contribution description
When there is only one interface we are simplifying a lot for the users
if the interface is set implicitly.
Issues/PRs references
"Fixes" the problems encountered in #8457.