[PATCH 2/3] ntdll/tests: Add tests for RtlIpv6StringToAddress (try 3)

[Mark Jansen]

Hey,

I do not have the intention to rewrite the ipv4 tests now, i do have
another few tests lined up for different ipv6 functions (addr2str),
i'll look into rewriting the ipv4 then.
As for now, few tests are better than no tests, right?

On Fri, Feb 20, 2015 at 11:47 AM, Stefan Dösinger
<stefandoesinger at gmail.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi,
>
> Thanks again for your work and patience! This goes in the right direction in my eyes. The full set of address tests are run against both versions, and it documents the differences between non-ex and ex better. Can you do the same with the ipv4 tests in patch 1?
>
> Am 2015-02-19 um 23:38 schrieb Mark Jansen:
>> +/* ipv6 addresses from https://github.com/beaugunderson/javascript-ipv6/tree/master/test/data */
>> +static const struct
>> +{
>> +    PCSTR address;
>> +    NTSTATUS res;
>> +    int terminator_offset;
>> +    int ip[8];
>> +    int broken;
> What is the meaning of broken here? Different behavior among different Windows versions? Or deviation from your source above?
Broken means xp pro, 2003, vista and 2008 did not behave correct.


>
>> +    int ex_fails;   /* 1 means ex should fail, 2 means skip for ex */
> Style hint: You can use an enum in this case.
Yeah, was a last minute change to add the 2.

>
> I guess you need the skip value because the last test fails on non-ex and succeeds on ex and your expected address does not match. Similarly, the behavior that the function writes to the output address even in case of a failure isn't exactly helpful.
>
> - From patch 3:
>> +        if (ipv6_tests[i].ex_fails == 2)    /* testing vs ex doesnt >make sense. */
>> +            continue;
> Please document why, it took me a while to come up with the guess above. (And I'm not even sure it's the real reason). You're also not checking the output IP for the IP tests from the non-ex tests here. Is there a reason for that? Are there any cases where -Ex returns a different IP than non-ex?
Because the function will succeed on ex, it is even in the ex-test table.
Cases like this are exactly why i didnt want it in one table in the first place.

>
>> +        if (ipv6_tests[i].ip[0] == -1)
>> +        {
>> +            for (j = 0; j < 8; ++j)
>> +                expected_ip.s6_words[j] = 0xabab;
>> +        }
> I guess you want to avoid writing 0xabab 8 times in the table. I don't have any objection, but I can't predict if Alexandre likes it - technically this is redundant code. I'd say keep it for now, if he doesn't like it he will tell you.
Indeed.
>
>> +            for (j = 0; j < 8; ++j)
>> +                expected_ip.s6_words[j] = ipv6_tests[i].ip[j];
> Would this do the right thing on big endian machines?
Probably not, but I was under the impression that windows doesnt run
on big endian machines.
If wine does run on big endian this might need an additional check,
however i do not have a b.e. machine to verify it on.
>
> About some individual tests:
>> +    { "1111:2222:3333:4444:5555:6666:7777::",           STATUS_SUCCESS,             34, { 0x1111, 0x2222, 0x3333, 0x4444, 0x5555, 0x6666, 0x7777, 0 }, 1 },
> Now that's an interesting corner case. Out of curiosity, what does "1111:2222:3333:4444:5555:6666:7777:" do in non-ex and ex (one : removed)?
>
>> +    { "::0:0:0:0:0:0:0",                                STATUS_SUCCESS,             13, { 0, 0, 0, 0, 0, 0, 0, 0 }, 0, 1 },
>> +    { "::0:a:b:c:d:e:f",                                STATUS_SUCCESS,             13, { 0, 0, 0, 0xa00, 0xb00, 0xc00, 0xd00, 0xe00 }, 0, 1 },
> Here it would be interesting to know what IP -Ex writes before it fails. I don't see why -Ex would fail if -Ex calls non-ex, given that the terminator points way beyond the problematic "::" at the start.
Ex doesnt write anything if it fails.
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iQIcBAEBAgAGBQJU5xDbAAoJEN0/YqbEcdMwmEIP/0ti3aPWMeI8yetig2eTIuAG
> 2QhNad7bgFgioJBremjKPDuDJViYhkv4XoIwie6bRFSF9VnEECmiv2wObhRz2e0E
> MgK1bGkZc0BXs4TiVoa/+JVxWgo3ddWn1Wpj+hyVRAvyEfj4NNfRWvcGaQ+cZmwu
> Sd0tx7BqCQLSPnNwyrl53nqVWjkkmARdSF2pLDMgXkwtGXlR2b2irYslYg+Sqgvb
> +ORAryKZvVLt8vL6wBclb8ruFWpro7/Rio6eZb8EfoRNH+8YdYWUrdfcoCE4Ui0i
> W2xsOnahZuPvuQR9id8zp4etIa+YrlGlVsIfHZrzDzCvbyuBlq+niSeWZxgNwMs+
> IUpHoHj0TYNUZ9A7PB16UPHowsPVE5GYvsdjFr4Rj8rHxQqvftpTwFxRqWuV4qkQ
> Qx+achRcfA/qXWWYHksENuQJxF29vIbgiLbpvW256A6zmsAvB/+LlqndeF+BuW4I
> 3rQ2H7ADBAMx8IscePSDNLAKd2vsYWj8gFlw7y10HnIE7/zedqtWPIaGocFnS0u5
> QR/ETfUW5uvOr4rqMcyMCxg3yhc/QvYV67LFMrLusJ1IlCBhcfAC4r2B91sECHXr
> fJvdfGwL6HBt3tAJ5zxXZjTEwiFc77TGxswbt4lQplhFqTHfi/ZovGUiS/W3nQl6
> vX0zFtoYLTRAK0KtCNiP
> =JRkZ
> -----END PGP SIGNATURE-----