[DTrace-devel] [PATCH 2/2] Support two representations for embedded IPv4 addresses

[Eugene Loh]

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?

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

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.

Maybe this weird difference between ntoa6 and ntop is just not worth 
solving (until we run out of other things to do)?