ntdll/tests: Add tests for RtlIpv4StringToAddressEx, RtlIpv6StringToAddress (resend)

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Please split the patch into one patch per tested function (ipv4ex, ipv6, ipv6ex).

Am 2015-02-13 um 22:35 schrieb Mark Jansen:
> +    struct
> +    {
> +        PCSTR address;
> +        NTSTATUS res;
> +        int ip[4];
> +        USHORT port;
> +    } tests[] =
You can make this const (or even static const, not much difference though).

> +        { "1.1.1.1:",       STATUS_INVALID_PARAMETER,   { 1, 1, 1, 1 }, 0xdead },
> +        { "1.1.1.1+",       STATUS_INVALID_PARAMETER,   { 1, 1, 1, 1 }, 0xdead },
> +        { "1.1.1.1:1",      STATUS_SUCCESS,             { 1, 1, 1, 1 }, 0x100 },
I recommend to add a few more tests:
"1.1.1.1" - without a port
"256.1.1.1:1" - out of range ip
"-1.1.1.1:1" - negative ip
"1.1.0.1" - There's a zero in it, should still be valid though.
Hex and octal IPs

> +    const int testcount = sizeof(tests) / sizeof(tests[0]);
> +    int i, Strict;
Technically i and testcount can be made unsigned. According to your function definition earlier in the file Strict should be a BOOLEAN. For consistency "Strict" should be renamed to lower case "strict".

> +    ok(res == STATUS_INVALID_PARAMETER,
> +           "[null address] res = 0x%08x, expected 0x%08x\n",
> +           res, STATUS_INVALID_PARAMETER);
> ...
> +    ok(res == tests[i].res,
> +       "[%s] res = 0x%08x, expected 0x%08x\n",
> +       tests[i].address, res, tests[i].res);
Indentation inconsistency in the line continuation.

> +                expected_ip.S_un.S_un_b.s_b1 = tests[i].ip[0];
> +                expected_ip.S_un.S_un_b.s_b2 = tests[i].ip[1];
> +                expected_ip.S_un.S_un_b.s_b3 = tests[i].ip[2];
> +                expected_ip.S_un.S_un_b.s_b4 = tests[i].ip[3];
Is there a reason why you didn't just put a IN_ADDR in the test structure? You'll probably need an extra curly brackets around each member of struct S_un_b and one for the union: {{1}, {1}, {1}, {1}}

Most of this applies in the same fashion to the ipv6 tests.

> +/* ipv6 addresses from https://github.com/beaugunderson/javascript-ipv6/tree/master/test/data */
This is licensed under MIT. IANAL, but I think it is ok to transfer to an LGPLv2 licensed project.

> +static void test_RtlIpv6StringToAddress(void)
> ...
> +    struct
> +    {
> +        PCSTR address;
> +        NTSTATUS res;
> +        int terminator_offset;
> +        int ip[8];
> +    } tests[] =
> ...
> +    struct
> +    {
> +        PCSTR address;
> +        NTSTATUS res;
> +        ULONG scope;
> +        USHORT port;
> +        int ip[8];
> +    } tests[] =
Did you try to share the data for the ipv6 and ipv6ex test?

Unfortunately my ipv6-foo is too limited to say much about the tests in the table :-( .

> +    /* sanity check */
> +    ok(sizeof(ip._S6_un) == (sizeof(USHORT)* 8), "sizeof(ip._S6_un)\n");
I think we have compile-time asserts for things like this, but I don't know how they work.

> +    res = pRtlIpv6StringToAddressExA("::", &ip, &scope, &port);
> +    ok(res == STATUS_SUCCESS, "[validate] res = 0x%08x, expected STATUS_SUCCESS\n", res);
> +    res = pRtlIpv6StringToAddressExA(NULL, &ip, &scope, &port);
> +    ok(res == STATUS_INVALID_PARAMETER, "[null string] res = 0x%08x, expected STATUS_INVALID_PARAMETER\n", res);
> +    res = pRtlIpv6StringToAddressExA("::", NULL, &scope, &port);
> +    ok(res == STATUS_INVALID_PARAMETER, "[null result] res = 0x%08x, expected STATUS_INVALID_PARAMETER\n", res);
> +    res = pRtlIpv6StringToAddressExA("::", &ip, NULL, &port);
> +    ok(res == STATUS_INVALID_PARAMETER, "[null scope] res = 0x%08x, expected STATUS_INVALID_PARAMETER\n", res);
> +    res = pRtlIpv6StringToAddressExA("::", &ip, &scope, NULL);
> +    ok(res == STATUS_INVALID_PARAMETER, "[null port] res = 0x%08x, expected STATUS_INVALID_PARAMETER\n", res);
Checking that the output variables remain unmodified in the ipv4 test was a good idea IMO. Please add it here as well. (It is somewhat redundant because the test table has invalid tests and checks if the output is untouched, but this wouldn't be the first place where we find strange error handling behavior...

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJU4Go3AAoJEN0/YqbEcdMwP/wP/j5pbxPuTt4dBpr/fxA/E/rv
H6+hoXqq3irBQWgIuU4sprfvR/VEcKYR7HnMvL8SX/aa7xvuFPSdZerjHGgpoz1R
SKlqg46X8HxIe6BvODUSpi90OrJ4UNmAutL2vhpFQVzSsblZ1xPkk3GKHnIubslC
fmUi9I91Vyk6Upvk4xI9FNyf7OddUcHkb+Xkw5qFCip/R4Mcad/lZ5lKq6pJWNhE
4VRIA19IlCshi0Z3CY1NM8ZdRCCg8Wi8MqP8jgBaHsR3yPfEoBa3qUjsB2ryhrlP
2ityxJSFgAl38OQ89kwwkL7KIxxrP7t8oDrjGpb76mPdnpWxRBNarmSSu+JAmDDg
jUlzsjftCifjTs2vZloDWbqqhEBY3a/SQ2tUHeIxuYplKpR/AN75WH7WxFOKpR1h
KgafrCMeCGi0s2dVymkqXNa8zluQ3nq/pC16nddelgXD4D3RMPwWBnEDY8bpspR7
n0SR2mMTDAt2mndeI8DXvSyyVE4umsl8sUCjWVpKE/lj2xKZjKyPWFVx8yv1omx+
EqF+RUA2Qld+HOCy7d2c00mPy0bNmthaj7T5tmQSjEVStmGgwSH0RytkrI1yqwU8
MyDotAIxZ3OAiHvMl6RHvq5pquWDM4FTJ+yMpdxiEKPIZ4dwMa5mkZvaCYWBB2Qv
NnxWITxzj7GvV1YH7U/5
=uuX8
-----END PGP SIGNATURE-----