posix: net_help: move inet_pton/inet_ntop completely to POSIX by miri64 · Pull Request #3612 · RIOT-OS/RIOT

miri64 · 2015-08-11T19:23:20Z

As promised to @OlegHahm in #3607: Moves inet_.to. completely to the posix headers. It uses the ng_ipv[46]_addr_(to|from)_str() functions and wraps them.

Depends on ~~#3608~~ (merged), ~~#3611~~ (merged) and ~~#3660~~ (merged).

Depends on ~~#3864~~ (merged) for external bug fixes.

BytesGalore · 2015-08-12T05:56:02Z

nice one, :)
I will test

BytesGalore · 2015-08-12T06:20:36Z

tests/unittests/tests-ipv4_addr/tests-ipv4_addr.c

how about adding a test for conversion with the inet_[pton|ntop](), like:

static void test_inet_pton_ntop__success(void) { ipv4_addr_t result; size_t res_str_len = 11; char res_str[res_str_len]; TEST_ASSERT_NOT_NULL(inet_pton(AF_INET, "0.1.12.123", &result)); TEST_ASSERT_NOT_NULL(inet_ntop(AF_INET, &result, res_str, res_str_len)); }

It would provide certainty that the wrappers work as intended (which is the case) and extend the coverage of these unittests for future CI tests in case the code changes.

I would rather add those to their own test suite, because while they are calling those functions in reality they have nothing to do with them.

ok, an own testsuite sounds reasonable

miri64 · 2015-08-18T19:29:03Z

Rebased to current master.

miri64 · 2015-08-19T00:52:52Z

Rebased to current master and dependencies and added #3660 as dependency.

miri64 · 2015-08-26T12:00:03Z

Rebased to current master and dependencies

miri64 · 2015-09-07T14:51:41Z

Rebased to current master and dependencies

miri64 · 2015-09-11T18:21:14Z

Squashed and rebased to current master (no more waiting for other PRs)

cgundogan · 2015-09-16T08:15:16Z

sys/posix/include/arpa/inet.h

This code does not compile. socklen_t is undefined here.

see miri64#21

cgundogan · 2015-09-16T12:49:01Z

tested with the gnrc_example. looks good. ACK

cgundogan · 2015-09-16T12:58:46Z

travis seems to be complaining

miri64 · 2015-09-16T13:28:55Z

Coap seems to be broken (not through this PR though as far as I can see). I get other errors in master.

miri64 · 2015-09-16T13:29:52Z

(the errors in this PR are unrelated to this PR, but are just different, since the headers are moved around here).

miri64 · 2015-09-16T13:30:20Z

On master I get:

make: Entering directory `/home/martine/Repositories/RIOT-OS/RIOT/tests/coap'
Building application "coap" for "native" with MCU "native".

"make" -C /home/martine/Repositories/RIOT-OS/RIOT/pkg/libcoap
"make" -C /home/martine/Repositories/RIOT-OS/RIOT/pkg/libcoap/libcoap
/home/martine/Repositories/RIOT-OS/RIOT/pkg/libcoap/libcoap/net.c:826:1: error: unused function 'check_opt_size'
      [-Werror,-Wunused-function]
check_opt_size(coap_opt_t *opt, unsigned char *maxpos) {
^
1 error generated.
make[2]: *** [/home/martine/Repositories/RIOT-OS/RIOT/tests/coap/bin/native/libcoap/net.o] Error 1
make[1]: *** [all] Error 2
make: *** [/home/martine/Repositories/RIOT-OS/RIOT/tests/coap/bin/native/libcoap.a] Error 2
make: Leaving directory `/home/martine/Repositories/RIOT-OS/RIOT/tests/coap'

miri64 · 2015-09-16T13:33:13Z

It seems to be only an issue with clang (which I used since I wasn't getting anything from gcc's cryptic error messages)

miri64 · 2015-09-16T13:37:03Z

Seems like the functions travis is complaining about are included in glibc and newlib, and thus what GCC is trying to say is, that these are redefinitons (avr and msp430 boards are blacklisted anyway for libcoap, so I'll just remove them for now).

miri64 · 2015-09-16T13:45:58Z

See #3864. Based this PR on that PR.

miri64 · 2015-09-17T00:36:47Z

Rebased and squashed

miri64 · 2015-09-17T01:23:44Z

Travis is happy. Does your ACK uphold @cgundogan?

cgundogan · 2015-09-17T09:08:35Z

yes 👍

posix: net_help: move inet_pton/inet_ntop completely to POSIX

miri64 assigned BytesGalore Aug 11, 2015

miri64 added this to the Release 2015.08 milestone Aug 11, 2015

miri64 force-pushed the posix/api/inet_ntop_inet_pton branch from bba4f50 to 8debc09 Compare August 11, 2015 19:24

miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Aug 11, 2015

BytesGalore reviewed Aug 12, 2015
View reviewed changes

miri64 force-pushed the posix/api/inet_ntop_inet_pton branch from 8debc09 to a74c65b Compare August 18, 2015 19:28

miri64 force-pushed the posix/api/inet_ntop_inet_pton branch from d8a0d6f to 290419b Compare August 19, 2015 00:52

miri64 force-pushed the posix/api/inet_ntop_inet_pton branch from 68f433f to 8bf8e44 Compare August 19, 2015 01:02

miri64 mentioned this pull request Aug 19, 2015

posix: redo socket API to use conn #3646

Merged

miri64 force-pushed the posix/api/inet_ntop_inet_pton branch from 8bf8e44 to 0440d3c Compare August 26, 2015 11:59

miri64 force-pushed the posix/api/inet_ntop_inet_pton branch from 0440d3c to 2770293 Compare September 7, 2015 14:51

miri64 force-pushed the posix/api/inet_ntop_inet_pton branch from 2770293 to 8a5500e Compare September 11, 2015 18:20

miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Sep 11, 2015

cgundogan reviewed Sep 16, 2015
View reviewed changes

cgundogan added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 16, 2015

miri64 mentioned this pull request Sep 16, 2015

posix: remove strcasecmp functions and macros #3864

Merged

miri64 force-pushed the posix/api/inet_ntop_inet_pton branch from 280f228 to fabdf35 Compare September 16, 2015 13:45

miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Sep 16, 2015

posix: net_help: move inet_pton/inet_ntop completely to POSIX

08a3f0b

miri64 force-pushed the posix/api/inet_ntop_inet_pton branch from fabdf35 to 08a3f0b Compare September 17, 2015 00:36

miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Sep 17, 2015

cgundogan added a commit that referenced this pull request Sep 17, 2015

Merge pull request #3612 from authmillenon/posix/api/inet_ntop_inet_pton

35df5b6

posix: net_help: move inet_pton/inet_ntop completely to POSIX

cgundogan merged commit 35df5b6 into RIOT-OS:master Sep 17, 2015

miri64 deleted the posix/api/inet_ntop_inet_pton branch September 17, 2015 09:09

Conversation

miri64 commented Aug 11, 2015

Uh oh!

BytesGalore commented Aug 12, 2015

Uh oh!

BytesGalore Aug 12, 2015

Choose a reason for hiding this comment

Uh oh!

miri64 Aug 12, 2015

Choose a reason for hiding this comment

Uh oh!

BytesGalore Aug 12, 2015

Choose a reason for hiding this comment

Uh oh!

miri64 commented Aug 18, 2015

Uh oh!

miri64 commented Aug 19, 2015

Uh oh!

miri64 commented Aug 26, 2015

Uh oh!

miri64 commented Sep 7, 2015

Uh oh!

miri64 commented Sep 11, 2015

Uh oh!

cgundogan Sep 16, 2015

Choose a reason for hiding this comment

Uh oh!

cgundogan Sep 16, 2015

Choose a reason for hiding this comment

Uh oh!

miri64 Sep 16, 2015

Choose a reason for hiding this comment

Uh oh!

cgundogan commented Sep 16, 2015

Uh oh!

cgundogan commented Sep 16, 2015

Uh oh!

miri64 commented Sep 16, 2015

Uh oh!

miri64 commented Sep 16, 2015

Uh oh!

miri64 commented Sep 16, 2015

Uh oh!

miri64 commented Sep 16, 2015

Uh oh!

miri64 commented Sep 16, 2015

Uh oh!

miri64 commented Sep 16, 2015

Uh oh!

miri64 commented Sep 17, 2015

Uh oh!

miri64 commented Sep 17, 2015

Uh oh!

cgundogan commented Sep 17, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants