Skip to content

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jun 19, 2018

Contribution description

This refactors the gnrc_sixlowpan_frag module for the API proposed
in #8511.

The ctx for gnrc_sixlowpan_frag_send() is required to be a
gnrc_sixlowpan_msg_frag_t object, so IPHC can later on use it to
provide the original datagram size (otherwise, we would need to adapt
the API just for that, which seems to me as convoluted as this
proposal).

I also provide an expose function with a future possibility to provide
more than just one gnrc_sixlowpan_msg_frag_t object later on (plus
having cleaner module separation in general).

Issues/PRs references

Addresses #8511 partially.

@miri64 miri64 added Area: network Area: Networking Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. GNRC labels Jun 19, 2018
@miri64 miri64 requested a review from cgundogan June 19, 2018 13:05
@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 19, 2018
@miri64 miri64 mentioned this pull request Jun 19, 2018
6 tasks
Copy link
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

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

Tested. ACK!

@cgundogan cgundogan added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 26, 2018
@cgundogan
Copy link
Member

Running './dist/tools/doccheck/check.sh' x
Command output:

	ERROR: Doxygen generates the following warnings:
	sys/include/net/gnrc/sixlowpan/frag.h:108: warning: argument 'context' of command @param is not found in the argument list of gnrc_sixlowpan_frag_recv(gnrc_pktsnip_t *pkt, void *ctx, unsigned page)
	sys/include/net/gnrc/sixlowpan/frag.h:115: warning: The following parameters of gnrc_sixlowpan_frag_recv(gnrc_pktsnip_t *pkt, void *ctx, unsigned page) are not documented:
	sys/include/net/gnrc/sixlowpan/frag.h:91: warning: argument 'context' of command @param is not found in the argument list of gnrc_sixlowpan_frag_send(gnrc_pktsnip_t *pkt, void *ctx, unsigned page)
	sys/include/net/gnrc/sixlowpan/frag.h:106: warning: The following parameters of gnrc_sixlowpan_frag_send(gnrc_pktsnip_t *pkt, void *ctx, unsigned page) are not documented:
Running './dist/tools/cppcheck/check.sh' x
Command output:

	sys/net/gnrc/network_layer/sixlowpan/frag/gnrc_sixlowpan_frag.c:231: warning (nullPointerRedundantCheck): Either the condition 'ctx!=0' is redundant or there is possible null pointer dereference: fragment_msg.

@miri64
Copy link
Member Author

miri64 commented Jun 26, 2018

@cgundogan except for squashing, Murdock is happy now. May I squash?

@cgundogan
Copy link
Member

@cgundogan except for squashing, Murdock is happy now. May I squash?

yup!

This refactors the `gnrc_sixlowpan_frag` module for the API proposed
in RIOT-OS#8511.

The `ctx` for `gnrc_sixlowpan_frag_send()` is required to be a
`gnrc_sixlowpan_msg_frag_t` object, so IPHC can later on use it to
provide the *original* datagram size (otherwise, we would need to adapt
the API just for that, which seems to me as convoluted as this
proposal).

I also provide an expose function with a future possibility to provide
more than just one `gnrc_sixlowpan_msg_frag_t` object later on (plus
having cleaner module separation in general).
@miri64 miri64 force-pushed the gnrc_sixlowpan_frag/enh/i8511 branch from aa9440f to a2eb3c7 Compare June 26, 2018 17:38
@miri64
Copy link
Member Author

miri64 commented Jun 26, 2018

Done

@cgundogan cgundogan merged commit 315c201 into RIOT-OS:master Jun 26, 2018
@miri64 miri64 deleted the gnrc_sixlowpan_frag/enh/i8511 branch June 26, 2018 17:54
@cladmi cladmi added this to the Release 2018.07 milestone Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants