ipv4_addr: initial import by miri64 · Pull Request #3608 · RIOT-OS/RIOT

miri64 · 2015-08-11T16:33:51Z

This provides just the type and string conversion functions.

OlegHahm · 2015-08-11T16:41:43Z

Re-assigned to @kaspar030, since he implemented some IPv4 stuff.

kaspar030 · 2015-08-11T17:01:48Z

Makefile.dep

Because it uses it for the IPv4 compatible addresses.

(I basically took those static functions out of there)

kaspar030 · 2015-08-11T17:06:04Z

Just FIY, in my code, I'll probably always use uint32_t to store and handle IPv4 addresses.

kaspar030 · 2015-08-11T17:09:11Z

Which means, IMHO this adds unnecessary cruft to net/ipv4.h. Same for ipv6.

miri64 · 2015-08-11T17:13:45Z

Main motivation for this module is to provide the wrapper promised in #3607. For this we need a pair of publicly available IPv4 string conversion function which this module provides.

miri64 · 2015-08-11T17:14:39Z

(and to full-fill the promise given in the comments of ipv6_addr_to_str.c/ipv6_addr_from_str() ;-))

miri64 · 2015-08-11T17:26:31Z

Addressed comments

miri64 · 2015-08-17T11:57:29Z

Rebased to current master

A-Paul · 2015-08-18T16:06:45Z

sys/net/network_layer/ipv4/addr/ipv4_addr_to_str.c

I'm wondering if the check for result_len makes any sense as it proves nothing regarding the length of result. And it isn't used for anything else like condition or loop control. So it may be removed completely?

It's a very fig-leaf security measurement I give you that, but as commented above: this code is not originally by myself.

Didn't want to blame anyone. :-)
And I overlooked that the function signature may not be changed.

miri64 · 2015-08-18T16:51:05Z

Addressed @A-Paul's comment

ipv6_addr: adapt doc to proposed changes in #3608

miri64 · 2015-08-18T21:50:23Z

Rebased to current master.

miri64 · 2015-08-26T11:51:35Z

Rebased to current master

kaspar030 · 2015-09-05T21:24:07Z

Could someone else review this? I don't like it - it has half as many lines as my whole IPv4 stack.

OlegHahm · 2015-09-06T21:38:17Z

@kaspar030, from what I can see most of the lines contain documentation or tests. The question why do we need this without support for IPv4?

haukepetersen · 2015-09-07T11:57:35Z

This PR looks ok to me, the only thing I strongly dislike is the dependency from IPv6 to IPv4! I would rather have this in optional: If the IPv4 module is used, then include the IPv4 encapsulation into IPv6 - so do not do this per default!

miri64 · 2015-09-07T12:17:40Z

@haukepetersen I know I repeat myself, but the inclusion of IPv4 is already part of the current master. This PR just exposes this for everyone to use. In the end this is a string conversion function and string conversion for IPv6 is already quite fat in itself (due to the 0-compression). IPv4 address conversion on the other hand is quite simple, so only a fraction of the comparably large IPv6 code. Since string conversion isn't meant to be in production code anyway (correct me if I'm wrong) I'm more in favor of having a few bytes more of code, just to handle legal (but arguably complicated) IPv6 addresses.

miri64 · 2015-09-07T12:19:58Z

Sorry, hit the wrong button.

miri64 · 2015-09-07T12:21:47Z

The question why do we need this without support for IPv4?

Because we want inet_pton/inet_ntop support independent from the old-stack-style net_helpmodule.

miri64 · 2015-09-07T12:51:20Z

Rebased

kaspar030 · 2015-09-07T13:00:13Z

@authmillenon Just a question - what boards are you using to test your code on?

miri64 · 2015-09-07T13:02:25Z

native, iotlab-m3 and samr21-xpro

miri64 · 2015-09-07T13:04:17Z

(plan to use msba2 again, as soon as cc11xx support is back)

kaspar030 · 2015-09-07T13:04:39Z

thx

OlegHahm · 2015-09-07T16:13:10Z

@kaspar030, how concrete is your plan to PR the nanostack anytime soon?

kaspar030 · 2015-09-07T18:36:00Z

Hey,

On 09/07/15 18:13, Oleg Hahm wrote:

@kaspar030, how concrete is your plan to PR the nanostack anytime soon?

When I find time, after the release. But it will probably come with it's
own, embedded-style, ipv4 shell support.

OlegHahm · 2015-09-11T14:30:04Z

ACK

miri64 · 2015-09-11T14:32:45Z

Fixed doc issue and squashed immediately.

cgundogan · 2015-09-11T14:54:57Z

Do you want to make this optional for module ipv6_addr instead of pulling it in automatically?

OlegHahm · 2015-09-11T14:59:36Z

Do you want to make this optional for module ipv6_addr instead of pulling it in automatically?

👍

miri64 · 2015-09-11T15:09:14Z

As a follow-up, yes.

miri64 · 2015-09-11T15:56:16Z

See #3822.

ipv4_addr: initial import

miri64 added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Aug 11, 2015

miri64 assigned OlegHahm Aug 11, 2015

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

OlegHahm assigned kaspar030 and unassigned OlegHahm Aug 11, 2015

kaspar030 reviewed Aug 11, 2015
View reviewed changes

This was referenced Aug 11, 2015

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

Merged

conn: Initial import of a light-weight network application API #3615

Merged

miri64 force-pushed the ipv4_addr/api/initial branch from 9755d6e to 01e01d5 Compare August 17, 2015 11:57

A-Paul reviewed Aug 18, 2015
View reviewed changes

miri64 added a commit to miri64/RIOT that referenced this pull request Aug 18, 2015

ipv6_addr: adapt doc to proposed changes in RIOT-OS#3608

391522e

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

Merge pull request #3656 from authmillenon/ipv6_addr/doc/fix

3ab2907

ipv6_addr: adapt doc to proposed changes in #3608

miri64 force-pushed the ipv4_addr/api/initial branch from 0bede36 to ee4875f Compare August 18, 2015 21:51

miri64 force-pushed the ipv4_addr/api/initial branch from ee4875f to b2b98c5 Compare August 26, 2015 11:51

kaspar030 removed their assignment Sep 5, 2015

miri64 closed this Sep 7, 2015

miri64 reopened this Sep 7, 2015

miri64 force-pushed the ipv4_addr/api/initial branch 2 times, most recently from f18a83a to b87376e Compare September 7, 2015 12:44

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

miri64 force-pushed the ipv4_addr/api/initial branch from b87376e to be587ca Compare September 11, 2015 14:32

miri64 added 2 commits September 11, 2015 16:32

ipv4_addr: initial import

db56507

tests: add unittests for ipv4_addr

be587ca

miri64 mentioned this pull request Sep 11, 2015

ipv6_addr: make IPv4 compatible addresses optional #3822

Merged

miri64 added a commit that referenced this pull request Sep 11, 2015

Merge pull request #3608 from authmillenon/ipv4_addr/api/initial

b9fcd2b

ipv4_addr: initial import

miri64 merged commit b9fcd2b into RIOT-OS:master Sep 11, 2015

miri64 deleted the ipv4_addr/api/initial branch September 11, 2015 18:03

Conversation

miri64 commented Aug 11, 2015

Uh oh!

OlegHahm commented Aug 11, 2015

Uh oh!

kaspar030 Aug 11, 2015

Choose a reason for hiding this comment

Uh oh!

miri64 Aug 11, 2015

Choose a reason for hiding this comment

Uh oh!

miri64 Aug 11, 2015

Choose a reason for hiding this comment

Uh oh!

kaspar030 commented Aug 11, 2015

Uh oh!

kaspar030 commented Aug 11, 2015

Uh oh!

miri64 commented Aug 11, 2015

Uh oh!

miri64 commented Aug 11, 2015

Uh oh!

miri64 commented Aug 11, 2015

Uh oh!

miri64 commented Aug 17, 2015

Uh oh!

A-Paul Aug 18, 2015

Choose a reason for hiding this comment

Uh oh!

miri64 Aug 18, 2015

Choose a reason for hiding this comment

Uh oh!

A-Paul Aug 19, 2015

Choose a reason for hiding this comment

Uh oh!

miri64 commented Aug 18, 2015

Uh oh!

miri64 commented Aug 18, 2015

Uh oh!

miri64 commented Aug 26, 2015

Uh oh!

kaspar030 commented Sep 5, 2015

Uh oh!

OlegHahm commented Sep 6, 2015

Uh oh!

haukepetersen commented Sep 7, 2015

Uh oh!

miri64 commented Sep 7, 2015

Uh oh!

miri64 commented Sep 7, 2015

Uh oh!

miri64 commented Sep 7, 2015

Uh oh!

miri64 commented Sep 7, 2015

Uh oh!

kaspar030 commented Sep 7, 2015

Uh oh!

miri64 commented Sep 7, 2015

Uh oh!

miri64 commented Sep 7, 2015

Uh oh!

kaspar030 commented Sep 7, 2015 via email

Uh oh!

OlegHahm commented Sep 7, 2015

Uh oh!

kaspar030 commented Sep 7, 2015

Uh oh!

OlegHahm commented Sep 11, 2015

Uh oh!

miri64 commented Sep 11, 2015

Uh oh!

cgundogan commented Sep 11, 2015

Uh oh!

OlegHahm commented Sep 11, 2015

Uh oh!

miri64 commented Sep 11, 2015

Uh oh!

miri64 commented Sep 11, 2015

Uh oh!

Reviewers