[PATCH 3/4] ws2_32/tests: Add comprehensive socket option validity tests

Alex Henrie alexhenrie24 at gmail.com
Wed Aug 11 13:02:18 CDT 2021


On Wed, Aug 11, 2021 at 9:18 AM Zebediah Figura (she/her)
<zfigura at codeweavers.com> wrote:
>
> This is a good idea in principle, but a few things bother me about it:
>
> (1) the tables are hard to read (could maybe be solved with indentation)?

I'd be happy to indent the tables, but honestly they would be a lot
cleaner if Wine always called SetLastError(WSAENOPROTOOPT) in
setsockopt when an option is not recognized. (It already sets
WSAENOPROTOOPT for SOL_SOCKET options, just not for network-level
options.) Can I just send a patch to do that, or do the error code
tests need to be added first?

> (2) it would probably make sense to split get/set into separate tables,
> and not to split tcp/udp? I dunno, looks potentially simpler.
>
> (3) It's a bit ambiguous in many cases whether an operation is supported
> or not. I.e. it'd be nice to have *successful* calls to most of these.
> At least WSAENOPROTOOPT is relatively unambiguous, but I can't tell if a
> given operation always returns WSAEINVAL (like, say, the hardcoded IPv4
> option 41) or only returns WSAEINVAL because we're not using the right
> arguments.
>
> Which is to say that maybe a table isn't that great after all :-/

In these tests, WSAEINVAL generally means that an option is supported
on TCP but not UDP or vice-versa.

Since the setsockopt tests depend on getting a valid result from
getsockopt, how about if we just skip calling setsockopt if we expect
getsockopt to fail? That would be a little less comprehensive, but the
tables would be a lot shorter.

> (4) It's also kind of unclear what /* win7 */ means: broken on win7, or
> before win7, or up to and including win7, or what?

Like in most other Wine tests, the comment tells the most recent
version of Windows where the option doesn't work correctly. For
example, /* win7 */ means that the option does not work on Windows 7,
but it works on Windows 8 and later.

By the way, the IPV6_RECVTCLASS patch has its own set of tests. Would
you approve it if I resend it by itself while we're still working on
the more comprehensive tabular tests?

Thanks for the feedback,

-Alex



More information about the wine-devel mailing list