libc: Import OpenBSD's inet_net_{ntop,pton}
Needs ReviewPublic
Actions

Authored by ivy on Fri, Sep 19, 8:12 PM.

Details

Reviewers

des
zlei
markj
glebius

Summary

Our versions of these functions (originally taken from BIND) simply
don't work correctly for AF_INET6. These were removed from BIND itself
quite a while ago, but OpenBSD has made quite a few fixes in the mean
time, so import their code.

Add tests for both functions.

MFC after: 1 week
Obtained from: OpenBSD (lib/libc/net)
Reported by: Nico Sonack <nsonack@herrhotzenplotz.de>
PR: 289198

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 67204
Build 64087: arc lint + arc unit

Event Timeline

ivy created this revision.Fri, Sep 19, 8:12 PM

Herald added a subscriber: imp. · View Herald TranscriptFri, Sep 19, 8:12 PM

ivy requested review of this revision.Fri, Sep 19, 8:12 PM

Harbormaster completed remote builds in B67179: Diff 162438.Fri, Sep 19, 8:12 PM

ivy added reviewers: zlei, markj.Fri, Sep 19, 8:14 PM

ivy added a reviewer: glebius.

inet_net_test: add a test for an overlong prefix

the comment says we want to test this, and we do, so actually test it.

Harbormaster completed remote builds in B67180: Diff 162439.Fri, Sep 19, 8:27 PM

add tests for invalid input

Harbormaster completed remote builds in B67181: Diff 162440.Fri, Sep 19, 8:32 PM

Oh, that's a lot to review :) Thanks a lot for creating the tests. Quick question: do the tests pass on the previous implementation?

In D52629#1202550, @glebius wrote:

do the tests pass on the previous implementation?

no. as the reporter mentioned in the PR, our current implementation is completely broken -- it almost never produces valid output. i'm not sure how no one noticed that, but i assume these functions just aren't very widely used (i'd never even heard of them until i looked at the PR).

fix a few nits in inet_net_test, and while here, add the full license text to the header (for legal reasons, my new preference for files i create)

Harbormaster completed remote builds in B67204: Diff 162492.Sat, Sep 20, 8:46 PM

This change touches inet_net_ntop_ipv4(). I think this deserves careful review.

Nothing uses these functions, they are non-standard and never worked properly. Even BIND (which is where we got them) didn't use them.

des added inline comments.Mon, Sep 22, 9:59 AM

lib/libc/inet/inet_net_ntop.c
73	This is wrong.

emaste added a subscriber: emaste.Tue, Oct 14, 2:55 PM

emaste added inline comments.Tue, Oct 14, 2:59 PM

lib/libc/inet/inet_net_pton.c
177	If these are currently broken and do not work we should just not support net classes.

ivy added inline comments.Wed, Oct 15, 11:03 AM

lib/libc/inet/inet_net_pton.c
177	this change is not related to network classes, which don't exist in IPv6 at all. as far as i'm aware our current implementation supports classes okay.