af: initial import of global AF_ definition header by miri64 · Pull Request #3660 · RIOT-OS/RIOT

miri64 · 2015-08-18T23:44:27Z

There are a lot of redefinitions of the AF_ macros. This PR once and for all defines them all in a central place that is not dependent from POSIX. I chose to define them in a similar fashion as they are defined in Linux: as an enum, that is also exposed to the preprocessor. This way the arbitrary numbering is left to the compiler, while the predecessor also can access these constants (and as a bonus can simply convert them to strings with the # operand).

miri64 · 2015-08-19T00:02:31Z

I removed most of the address families defined in socket_base/socket.h because I really don't expect them to be implemented in RIOT ever (I'm not even that optimistic about AF_UNIX, but at least that is required in the POSIX specification).

I also removed the PF_ macros, since they seem kind of redundant to me and also are not mentioned in the POSIX specification. NHDP was the only reference of them so I changed it to the equivalent AF_ macro.

BytesGalore · 2015-08-19T06:39:06Z

sys/net/include/socket_base/socket.h

OT: the net/ipv6 and net/ng_ipv6 will get merged to net/ipv6 soon-ish right?

No net/ipv6 was taken out of GNRC and is independent of it (it contains general definitions like addresses, headers and related functions). net/ng_ipv6.h was moved to net/gnrc/ipv6.h yesterday.

cgundogan · 2015-08-19T06:40:47Z

OT: the net/ipv6 and net/ng_ipv6 will get merged to net/ipv6 soon-ish right?

AFAIU, net/ipv6 is more a common base for all ipv6 stacks, while net/ng_ipv6, or possibly in the future net/gnrc_ipv6 is pulling in net/ipv6

BytesGalore · 2015-08-19T06:48:50Z

@cgundogan ok, I misinterpreted it as part/step of the migration from the old stack.

BytesGalore · 2015-08-19T06:50:39Z

sys/include/net/af.h

ok, really nitpicking: too much newlines

miri64 · 2015-08-19T08:50:45Z

@cgundogan ok, I misinterpreted it as part/step of the migration from the old stack.

The old stack is gone since about 2 weeks or so. The only things left are a socket_base skeleton, which will be hopefully removed as soon as the new connection API was removed, and net_help.

BytesGalore · 2015-08-20T06:46:51Z

pkg/oonf_api/0001-add-RIOT-support.patch

I didn't tested with oonf yet, but doesn't removing the both includes will harm using this package?

I did not test it either, but judging from the diff it looked like those were only included for the AF_ macros.

Seems to be broken in master.

Reworked the patch file.

Seems to be broken in master.

No I'm just too stupid to use it ;-P

BytesGalore · 2015-08-24T07:32:58Z

needs rebase :(

miri64 · 2015-08-24T07:46:12Z

Done.

BytesGalore · 2015-08-24T07:59:42Z

nice thx :)

seems we need AF_MAX in the enums [1]:

In file included from RIOT/pkg/oonf_api/oonf_api/src-api/common/avl_comp.h:46:0,
                 from RIOT/pkg/oonf_api/oonf_api/src-api/common/avl_comp.c:45:
RIOT/pkg/oonf_api/oonf_api/src-api/common/netaddr.h:74:14: error: ‘AF_MAX’ undeclared here (not in a function)
   AF_MAC48 = AF_MAX + 1,
              ^

[1] https://github.com/RIOT-OS/RIOT/pull/3660/files#diff-712c838674c0f484091fcc0da054fea7R40

BytesGalore · 2015-08-24T08:18:57Z

ok, since we have no socket_base module the oonf [1] cannot be used currently.
I updated #3417 accordingly.

[1] https://github.com/RIOT-OS/RIOT/blob/master/Makefile.dep#L228

miri64 · 2015-08-24T08:50:39Z

I could have told you that (but failed to update the issue too).

BytesGalore · 2015-08-24T08:57:34Z

np :)
It seems that all is caught now, and all required AF_ are available.
Though, there's an unusual AF_CC110X definition for NHDP [1]

[1] https://github.com/RIOT-OS/RIOT/blob/master/sys/net/routing/nhdp/nhdp.h#L104

BytesGalore · 2015-08-24T09:01:22Z

I would say please squash and lets see what travis says

miri64 · 2015-08-24T11:12:03Z

Though, there's an unusual AF_CC110X definition for NHDP [1]

Saw it, though that it was implementation dependent and shrugged it off :D

miri64 · 2015-08-24T11:13:55Z

Squashed

BytesGalore · 2015-08-24T13:31:00Z

nice, I would say ACK

af: initial import of global AF_ definition header

OlegHahm · 2015-08-25T09:32:44Z

pkg/oonf_api/0001-add-RIOT-support.patch

How did these additional white trailing spaces passed the CI?

Some versions of git (my local e.g.) register patch files as binary files and thus ignore their content for patching, diff and whitespace checks.

miri64 added Area: POSIX Area: POSIX API wrapper Area: network Area: Networking Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Aug 18, 2015

miri64 assigned BytesGalore Aug 18, 2015

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

miri64 force-pushed the af/api/initial branch 2 times, most recently from e2d3712 to ce51fbd Compare August 18, 2015 23:46

This was referenced Aug 19, 2015

posix: net_help: move inet_pton/inet_ntop completely to POSIX #3612

Merged

posix: redo socket API to use conn #3646

Merged

BytesGalore reviewed Aug 19, 2015
View reviewed changes

BytesGalore reviewed Aug 20, 2015
View reviewed changes

miri64 force-pushed the af/api/initial branch from 93a819f to d784fcd Compare August 20, 2015 08:09

OlegHahm added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Aug 20, 2015

miri64 force-pushed the af/api/initial branch from d784fcd to c6010a3 Compare August 24, 2015 07:46

miri64 force-pushed the af/api/initial branch from 481a7d7 to ac0e2f4 Compare August 24, 2015 11:13

miri64 added 2 commits August 24, 2015 13:13

af: initial import of global AF_ definition header

ecb2bb5

net: remove redefinitions and their dependencies throughout the stack

ac0e2f4

miri64 added a commit that referenced this pull request Aug 25, 2015

Merge pull request #3660 from authmillenon/af/api/initial

8820804

af: initial import of global AF_ definition header

miri64 merged commit 8820804 into RIOT-OS:master Aug 25, 2015

miri64 deleted the af/api/initial branch August 25, 2015 07:14

OlegHahm reviewed Aug 25, 2015
View reviewed changes

Conversation

miri64 commented Aug 18, 2015

Uh oh!

miri64 commented Aug 19, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgundogan commented Aug 19, 2015

Uh oh!

BytesGalore commented Aug 19, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miri64 commented Aug 19, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BytesGalore commented Aug 24, 2015

Uh oh!

miri64 commented Aug 24, 2015

Uh oh!

BytesGalore commented Aug 24, 2015

Uh oh!

BytesGalore commented Aug 24, 2015

Uh oh!

miri64 commented Aug 24, 2015

Uh oh!

BytesGalore commented Aug 24, 2015

Uh oh!

BytesGalore commented Aug 24, 2015

Uh oh!

miri64 commented Aug 24, 2015

Uh oh!

miri64 commented Aug 24, 2015

Uh oh!

BytesGalore commented Aug 24, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants