Skip to content

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jul 30, 2021

Contribution description

This provides a function to uri_parser to split the query part of a given URI parsing result into its components.

Testing procedure

tests/unittests were amended for the new function, so

make -C tests/unittests/ tests-uri_parser test

should succeed for a board of choice.

Issues/PRs references

None.

@miri64 miri64 added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Jul 30, 2021
@miri64 miri64 added this to the Release 2021.10 milestone Jul 30, 2021
@miri64 miri64 requested a review from cgundogan July 30, 2021 12:45
@github-actions github-actions bot added Area: sys Area: System Area: tests Area: tests and testing framework labels Jul 30, 2021
* In that case, the array is filled with the first @p params_len
* name-value-pairs in uri_parser_result_t::query of @p uri_parsed.
*/
int uri_parser_split_query(const uri_parser_result_t *uri_parsed,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use a uri_parser_result_t * here when you are only interested in the query element?
This function could be more generic:

Suggested change
int uri_parser_split_query(const uri_parser_result_t *uri_parsed,
int uri_parser_split_query(const char *query, size_t query_len,

Copy link
Member Author

Choose a reason for hiding this comment

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

Since then I need to also add verification of the query string to this function, which is already done in uri_parser_process_string()

Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

My only concerns were about the API, but you explained those - the code looks good.
Please squash!

@miri64 miri64 force-pushed the uri_parser/enh/query-split branch from 8b3c2ad to 4d2a942 Compare August 4, 2021 10:47
@miri64
Copy link
Member Author

miri64 commented Aug 4, 2021

Squashed and rebased

@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 Aug 4, 2021
@miri64 miri64 merged commit 5c2b599 into RIOT-OS:master Aug 4, 2021
@miri64 miri64 deleted the uri_parser/enh/query-split branch August 4, 2021 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants