On 3/4/22 20:36, Zebediah Figura wrote:
+ /* Test option short length. */ + for (i = 0; i < ARRAY_SIZE(test_optsize); ++i) + { + s2 = socket( test_optsize[i].af, test_optsize[i].type, 0 ); + ok(s2 != INVALID_SOCKET, "socket() failed error %d\n", WSAGetLastError()); + for (j = 0; j < 3; ++j)
ARRAY_SIZE(test_optsize[i].sizes)?
The way this test is structured does not make it very easy to read, but I'm struggling to find a better option. The individual sockopts seem very idiosyncratic...
Yes, those behaviour details don't seem to fit any obvious pattern. Maybe I can make it a bit easier if always code the values returned from getsockopt() instead of setting the "default" cases in the code?
Finding a way to include TCP_NODELAY in this table, as well as finding a way to test lengths less than 1 and greater than 4, would probably be helpful. I don't know if you have those in your pending patches.
I am introducing TCP_NODELAY in the same table later, yeah. The reason that it is not here immediately is that TCP_NODELAY needs output length fixup in getsockopt() not to introduce obsucre todo_wine and the remove it, thus a separate patch.
I tested greater sizes separately for some options, I will add some tests (hopefully they are all work the same as size 4, at least those that I tried did). Didn't try length < 0 but I will.
+ { + DWORD expected_last_error, expected_value, expected_size, save_value; + int expected_err;
+ winetest_push_context("i %u, level %d, optname %d, len %u", + i, test_optsize[i].level, test_optsize[i].optname, 1 << j);
+ size = sizeof(save_value); + err = getsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char*)&save_value, &size); + ok(!err, "Unexpected getsockopt result %d.\n", err);
+ if (test_optsize[i].accepts_short_len || j == 2)
"j == 2" is not great, maybe "size = 1 << j;" earlier, and then "size == 4" here, would be clearer.
+ { + expected_err = 0; + expected_last_error = ERROR_SUCCESS; + expected_size = test_optsize[i].sizes[j] ? test_optsize[i].sizes[j] : 4; + if (test_optsize[i].values[j]) + expected_value = test_optsize[i].values[j]; + else if (expected_size == 4) + expected_value = 1; + else + expected_value = (0xdeadbeef & ~((1 << expected_size * 8) - 1)) | 1;
This is not exactly easy to read. Maybe something like the following would be clearer?
value = 1; expected_value = 0xdeadbeef; memcpy(&expected_value, &value, expected_size);
Yeah, sure.