Skip to content

sys/net/netutils: add netutils_get_ipv4()#17764

Merged
benpicco merged 2 commits intoRIOT-OS:masterfrom
HendrikVE:pr/netutils_netutils_get_ipv4
Mar 9, 2022
Merged

sys/net/netutils: add netutils_get_ipv4()#17764
benpicco merged 2 commits intoRIOT-OS:masterfrom
HendrikVE:pr/netutils_netutils_get_ipv4

Conversation

@HendrikVE
Copy link
Contributor

@HendrikVE HendrikVE commented Mar 7, 2022

Contribution description

Add function netutils_get_ipv4() to netutils.

Testing procedure

Existing test in tests/netutils was extended.

Can be tested with #17763

@miri64
Copy link
Member

miri64 commented Mar 7, 2022

After #16688 was merged I tried to port the examples to used URIs (utilizing the uri_parser) module, which was halted, since netutils_get_ipv6() requires a string (\0 terminated char array) and not a char buffer (a char array + length), the latter being the output of the non-destructive uri_parser. Maybe in the design of netutils_get_ipv4() this lack in the original netutils_get_ipv6() can already be addressed?

@HendrikVE
Copy link
Contributor Author

After #16688 was merged I tried to port the examples to used URIs (utilizing the uri_parser) module, which was halted, since netutils_get_ipv6() requires a string (\0 terminated char array) and not a char buffer (a char array + length), the latter being the output of the non-destructive uri_parser. Maybe in the design of netutils_get_ipv4() this lack in the original netutils_get_ipv6() can already be addressed?

Unfortunately it can't, because the root of the problem is that sock_dns_query, which is used by netutils_get_ipv4 and netutils_get_ipv6, requires a null-terminated string .

@miri64
Copy link
Member

miri64 commented Mar 7, 2022

Unfortunately it can't, because the root of the problem is that sock_dns_query, which is used by netutils_get_ipv4 and netutils_get_ipv6, requires a null-terminated string .

That sounds also like a fixable problem ;-), but I can understand if you are not willing to do it here. :-)

@HendrikVE
Copy link
Contributor Author

Yeah I think that's something for another PR, but I can add it to my list ;)

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.

Looks good to me. Let's not go down a rabbit hole for adding some simple functionality.
Just some small nitpicks:

@HendrikVE HendrikVE force-pushed the pr/netutils_netutils_get_ipv4 branch from 1b0bc98 to ffd93c9 Compare March 8, 2022 12:35
@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 Mar 8, 2022
@benpicco benpicco merged commit f83fa88 into RIOT-OS:master Mar 9, 2022
@HendrikVE HendrikVE deleted the pr/netutils_netutils_get_ipv4 branch March 9, 2022 17:18
@OlegHahm OlegHahm added this to the Release 2022.04 milestone Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants