[PATCH 2/4] ws2_32/tests: Test ICMP.

[Zebediah Figura]

On 7/5/22 14:53, Paul Gofman wrote:
> From: Paul Gofman <pgofman at codeweavers.com>
> 
> Signed-off-by: Paul Gofman <pgofman at codeweavers.com>
> ---
>   dlls/ws2_32/tests/sock.c | 150 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 150 insertions(+)
> 

I don't have much familiarity with ICMP, but...

> +static void test_icmp(const char *dest_addr)

Any reason to pass this as a parameter? I'm guessing it's convenient to 
test ping over a real adapter, but presumably it can also just be edited 
inside the function...

> +{
> +    static const unsigned int ping_data = 0xdeadbeef;
> +    struct icmp_hdr *icmp_h;
> +    BYTE send_buf[sizeof(struct icmp_hdr) + sizeof(ping_data)];
> +    UINT16 recv_checksum, checksum;
> +    unsigned int reply_data;
> +    struct sockaddr_in sa;
> +    struct ip_hdr *ip_h;
> +    struct in_addr addr;
> +    BYTE recv_buf[256];
> +    SOCKET s;
> +    int ret;
> +
> +    s = WSASocketA(AF_INET, SOCK_RAW, IPPROTO_ICMP, NULL, 0, 0);
> +    if (s == INVALID_SOCKET)
> +    {
> +        ret = WSAGetLastError();
> +        ok(ret == WSAEACCES, "Expected 10013, received %d\n", ret);
> +        skip("SOCK_RAW is not supported\n");
> +        return;
> +    }

To clarify, this should never trip on Windows machines; it's only here 
for Linux machines that don't support unprivileged ICMP, right? If so a 
comment to that effect would be helpful. I also don't know if we want to 
use a todo_wine in that case instead of skip().

> +
> +    icmp_h = (struct icmp_hdr *)send_buf;
> +    icmp_h->type = ICMP4_ECHO_REQUEST;
> +    icmp_h->code = 0;
> +    icmp_h->checksum = 0;
> +    icmp_h->un.echo.id = 0xbeaf; /* will be overwritten for linux ping socks */
> +    icmp_h->un.echo.sequence = 1;
> +    *(unsigned int *)(icmp_h + 1) = ping_data;
> +    icmp_h->checksum = chksum(send_buf, sizeof(send_buf));
> +
> +    memset(&sa, 0, sizeof(sa));
> +    sa.sin_family = AF_INET;
> +    sa.sin_port = 0;
> +    sa.sin_addr.s_addr = inet_addr(dest_addr);
> +
> +    ret = sendto(s, (char *)send_buf, sizeof(send_buf), 0, (struct sockaddr*)&sa, sizeof(sa));
> +    ok(ret != SOCKET_ERROR, "got error %d.\n", WSAGetLastError());

Any reason not to just say "ok(ret == sizeof(send_buf)"?