On Fri, Jul 26, 2019 at 7:25 PM Peter Geoghegan <pg@bowt.ie> wrote:
> I guess that the idea here was to prevent masking on ipv6 addresses,
> though not on ipv4 addresses. Obviously we're only dealing with a
> prefix with ipv6 addresses, whereas we usually have the whole raw
> ipaddr with ipv4. Not sure if I'm doing the right thing there in v3,
> even though the tests pass. In any case, this will need to be a lot
> clearer in the final version.
This turned out to be borked for certain IPv6 cases, as suspected.
Attached is a revised v6, which fixes the issue by adding the explicit
handling needed when ipaddr_datum is just a prefix of the full ipaddr
from the authoritative representation. Also made sure that the tests
will catch issues like this. Separately, it occurred to me that it's
probably not okay to do straight type punning of the ipaddr unsigned
char array to a Datum on alignment-picky platforms. Using a memcpy()
seems like the right approach, which is what we do in the latest
revision.
I accepted almost all of Brandur's comment revisions from v5 for v6.
I'm probably going to commit this tomorrow morning Pacific time. Do
you have any final input on the testing, Brandur? I would like to hear
your thoughts on the possibility of edge cases that still don't have
coverage. The tests will break if the new "if (ip_bits(authoritative)
== 0)" branch is removed, but only at one exact point. I'm pretty sure
that there are no remaining subtleties like that one, but two heads
are better than one.
--
Peter Geoghegan