Skip to content
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.

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

nanocoap: add coap_get_token() #17976

Merged
merged 3 commits into from
Apr 22, 2022
Merged

Conversation

benpicco
Copy link
Contributor

Contribution description

This adds a coap_get_token() function so we can eventually get rid of the token pointer in coap_pkt_t.

Testing procedure

Issues/PRs references

@benpicco benpicco requested a review from kfessel April 21, 2022 12:51
@github-actions github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System labels Apr 21, 2022
@benpicco benpicco requested a review from maribu April 21, 2022 12:52
Copy link
Contributor

@kfessel kfessel left a comment

Choose a reason for hiding this comment

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

 * @returns     pointer to the token-position

does not tell you about its lenght

sys/include/net/nanocoap.h Outdated Show resolved Hide resolved
sys/net/application_layer/gcoap/forward_proxy.c Outdated Show resolved Hide resolved
sys/include/net/nanocoap.h Outdated Show resolved Hide resolved
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 21, 2022
@benpicco benpicco requested a review from chrysn April 21, 2022 14:23
@fjmolinas
Copy link
Contributor

Anything missing @kfessel?

Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

ACK. This is a nice one :)

@benpicco benpicco merged commit c032e40 into RIOT-OS:master Apr 22, 2022
@benpicco benpicco deleted the coap_get_token branch April 22, 2022 08:08
@kfessel
Copy link
Contributor

kfessel commented Apr 22, 2022

I am not sure about the name coap_get_token sounds a bit like you get a value so:
coap_token
coap_pkt_token
coap_pkt_token_ptr
coap_token_ptr

functionality is good (and marian just acked)

@benpicco
Copy link
Contributor Author

We also have coap_get_token_len() so it fits right in.

@kfessel
Copy link
Contributor

kfessel commented Apr 22, 2022

with coap_get_token_len() you get a value (not a ptr into the pktbuffer) (i seems kind o strange to have a get on the left side of a memcpy)

but maybe i am over thinking that naming thing,

lets move on an deprecate the use of coap_pkt_t.token

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants