[DTrace-devel] [PATCH 2/2] Support two representations for embedded IPv4 addresses
Kris Van Hees
kris.van.hees at oracle.com
Fri Sep 15 23:36:54 UTC 2023
On Fri, Sep 15, 2023 at 05:30:43PM -0400, Eugene Loh via DTrace-devel wrote:
> Yipes. Once again, I didn't get the email in my inbox. Yick.
>
> Anyhow, the big question is whether we are changing the interface. The
> commit message says the patch is introducing an undocumented arg. Well, the
> test suite has a number of tests that check that not too many args are being
> passed to a function. (Not every function is so tested, but arguably that's
> a short-coming of the test suite rather than an intentional policy of
> allowing too many args for all those untested functions.) So, it seems to
> me that either we are changing the inet_ntoa6() interface (and should
> document that) or we are breaking a test (that we to date have declined to
> write). Is it legal for a D user to write inet_ntop(af, addr, 0)? How
> about inet_ntop(af, addr, 0, 0)? How should D handle those two cases? Or
> are we using an optional arg that a D user will not be able to use?
We are not changing the interface in a substantial way. We are extending it
a tiny bit by allowing inet_ntoa6(addr, int). If int is 0, the regular
inet_ntoa6() behaviour results (no :: prefix) whereas if int is not 0, then
we yield a result with :: prefix.
So, this is a completely backwards compatible change, and if anyone wants to
use the new feature, they certainly can.
I have no problem documenting it, but I also do not see that there is any
issue if this remains undocumented until documentation gets updated for other
things as well.
I also do not mind adding tests that ensure that only two arguments can be
supplied, and that the proper behaviour results from not supplying this 2nd
argument, or from supplying 0 vs non-0 values.
I do not see any reason why we would prevent users from using this, nor is
that really something we can easily do.
> Incidentally,
>
> +.Lipv4_2:
> + /* Output IPv4 address prefixed by ::ffff: (if is_ntop is set). */
> + ldxw %r0, [%fp + -8] /* restore is_ntop */
> + jeq %r0, 0, .Lipv4
> + stb [%r9 + 0], ':'
> + stb [%r9 + 1], ':'
> + add %r9, 2
> + mov %r1, -1
> + mov %r2, %r9
> + call write_hex16
> + add %r9, %r0
> + stb [%r9 + 0], ':'
> + add %r9, 1
> + ja .Lipv4
> +
> .Lipv4:
>
> No need for the ja since you fall through to that label anyhow. Also,
> instead of the "add %r9,..." instructions and everything in-between, how
> about simply
True, good point.
> stb [%r9+2], 'f'
> stb [%r9+3], 'f'
> stb [%r9+4], 'f'
> stb [%r9+5], 'f'
> stb [%r9+6], ':'
> add %r9, 7
>
> Fewer instructions, easier to read.
Also on this.
> Maybe this weird difference between ntoa6 and ntop is just not worth solving
> (until we run out of other things to do)?
That becomes a bit irrelevant given that it is already implemented...
More information about the DTrace-devel
mailing list