-----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...