[PATCH 3/8] [v2] wsdapi: Add support for sending messages via UDP multicast.
Huw Davies
huw at codeweavers.com
Mon Mar 19 05:43:32 CDT 2018
On Thu, Mar 15, 2018 at 09:58:09PM +0000, Owen Rudge wrote:
> I've replaced the string address conversion with static numeric addresses,
> as suggested.
>
> I have kept the send_message algorithm as it was before as it more closely
> reflects the proscribed algorithm in the SOAP spec, and the delay
> calculations do differ between the two. I think trying to alter it to reduce
> the number of lines would make the algorithm less clear.
>
There are still some very long lines in this patch (and others in the
series). Personally, I find anything over c.120 hard to follow.
> +BOOL send_udp_multicast_of_type(char *data, int length, int max_initial_delay, ULONG family)
> +{
> + IP_ADAPTER_ADDRESSES *adapter_addresses = NULL, *adapter_addr;
> + sending_thread_params *send_params;
> + ULONG bufferSize = 0;
> + const struct in6_addr i_addr_zero = {0};
Any reason why you didn't use static const here? Then you don't need the
explicit initialization, which clang wants to be {{0}} anyway.
> + LPSOCKADDR sockaddr;
> + BOOL ret = FALSE;
> + HANDLE thread_handle;
> + const char ttl = 8;
> + ULONG retval;
> + SOCKET s;
> +
> + /* Get size of buffer for adapters */
> + retval = GetAdaptersAddresses(family, 0, NULL, NULL, &bufferSize);
> +
> + if (retval != ERROR_BUFFER_OVERFLOW)
> + {
> + WARN("GetAdaptorsAddresses failed with error %08x\n", retval);
> + goto cleanup;
> + }
> +
> + adapter_addresses = (IP_ADAPTER_ADDRESSES *) heap_alloc(bufferSize);
> +
> + if (adapter_addresses == NULL)
> + {
> + WARN("Out of memory allocating space for adapter information\n");
> + goto cleanup;
> + }
> +
> + /* Get list of adapters */
> + retval = GetAdaptersAddresses(family, 0, NULL, adapter_addresses, &bufferSize);
> +
> + if (retval != ERROR_SUCCESS)
> + {
> + WARN("GetAdaptorsAddresses failed with error %08x\n", retval);
> + goto cleanup;
> + }
> +
> + for (adapter_addr = adapter_addresses; adapter_addr != NULL; adapter_addr = adapter_addr->Next)
> + {
> + if (adapter_addr->FirstUnicastAddress == NULL)
> + {
> + TRACE("No address found for adaptor '%s' (%p)\n", debugstr_a(adapter_addr->AdapterName), adapter_addr);
> + continue;
> + }
> +
> + sockaddr = adapter_addr->FirstUnicastAddress->Address.lpSockaddr;
> +
> + /* Create a socket and bind to the adapter address */
> + s = socket(family, SOCK_DGRAM, IPPROTO_UDP);
> +
> + if (s == INVALID_SOCKET)
> + {
> + WARN("Unable to create socket: %d\n", WSAGetLastError());
> + continue;
> + }
> +
> + if (bind(s, sockaddr, adapter_addr->FirstUnicastAddress->Address.iSockaddrLength) == SOCKET_ERROR)
> + {
> + WARN("Unable to bind to socket (adaptor '%s' (%p)): %d\n", debugstr_a(adapter_addr->AdapterName), adapter_addr, WSAGetLastError());
> + closesocket(s);
> + continue;
> + }
> +
> + /* Set the multicast interface and TTL value */
> + setsockopt(s, IPPROTO_IP, IP_MULTICAST_IF, (char *) &i_addr_zero, (family == AF_INET6) ? sizeof(struct in6_addr) : sizeof(struct in_addr));
> + setsockopt(s, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl));
> +
> + /* Set up the thread parameters */
> + send_params = heap_alloc(sizeof(*send_params));
> +
> + send_params->data = heap_alloc(length);
> + memcpy(send_params->data, data, length);
> + send_params->length = length;
> + send_params->sock = s;
> + send_params->max_initial_delay = max_initial_delay;
> +
> + memset(&send_params->dest, 0, sizeof(SOCKADDR_STORAGE));
> + send_params->dest.ss_family = family;
> +
> + if (family == AF_INET)
> + {
> + SOCKADDR_IN *sockaddr4 = (SOCKADDR_IN *)&send_params->dest;
> +
> + sockaddr4->sin_port = htons(SEND_PORT);
> + sockaddr4->sin_addr.S_un.S_addr = htonl(SEND_ADDRESS_IPV4);
> + }
> + else
> + {
> + SOCKADDR_IN6 *sockaddr6 = (SOCKADDR_IN6 *)&send_params->dest;
> +
> + sockaddr6->sin6_port = htons(SEND_PORT);
> + memcpy(&sockaddr6->sin6_addr, &send_address_ipv6, sizeof(send_address_ipv6));
> + }
> +
> + thread_handle = CreateThread(NULL, 0, sending_thread, send_params, 0, NULL);
I wonder whether we really want a new thread / message. This should probably be moved
to a thread pool api, but perhaps that can happen later on.
> +
> + if (thread_handle == NULL)
> + {
> + WARN("CreateThread failed (error %d)\n", GetLastError());
> + closesocket(s);
> +
> + heap_free(send_params->data);
> + heap_free(send_params);
> +
> + continue;
> + }
> +
> + CloseHandle(thread_handle);
> + }
> +
> + ret = TRUE;
> +
> +cleanup:
> + if (adapter_addresses != NULL) heap_free(adapter_addresses);
> +
> + return ret;
> +}
> +
> +BOOL send_udp_multicast(IWSDiscoveryPublisherImpl *impl, char *data, int length, int max_initial_delay)
> +{
> + if ((impl->addressFamily & WSDAPI_ADDRESSFAMILY_IPV4) && (!send_udp_multicast_of_type(data, length, max_initial_delay, AF_INET))) return FALSE;
> + if ((impl->addressFamily & WSDAPI_ADDRESSFAMILY_IPV6) && (!send_udp_multicast_of_type(data, length, max_initial_delay, AF_INET6))) return FALSE;
> + return TRUE;
> +}
> +
> +void terminate_networking(IWSDiscoveryPublisherImpl *impl)
> +{
> + BOOL needsCleanup = impl->publisherStarted;
> +
> + impl->publisherStarted = FALSE;
> +
> + if (needsCleanup)
> + WSACleanup();
> +}
> +
> +BOOL init_networking(IWSDiscoveryPublisherImpl *impl)
> +{
> + WSADATA wsaData;
> + int ret = WSAStartup(MAKEWORD(2, 2), &wsaData);
> +
> + if (ret != 0)
> + {
> + WARN("WSAStartup failed with error: %d\n", ret);
> + return FALSE;
> + }
> +
> + impl->publisherStarted = TRUE;
> +
> + /* TODO: Start listening */
> + return TRUE;
> +}
> diff --git a/dlls/wsdapi/soap.c b/dlls/wsdapi/soap.c
> index efc063ca05..bd73c73acb 100644
> --- a/dlls/wsdapi/soap.c
> +++ b/dlls/wsdapi/soap.c
> @@ -27,12 +27,15 @@
> #include "wine/debug.h"
> #include "wine/heap.h"
>
> +WINE_DEFAULT_DEBUG_CHANNEL(wsdapi);
> +
> #define APP_MAX_DELAY 500
>
> static HRESULT write_and_send_message(IWSDiscoveryPublisherImpl *impl, WSD_SOAP_HEADER *header, WSDXML_ELEMENT *body_element,
> struct list *discovered_namespaces, IWSDUdpAddress *remote_address, int max_initial_delay)
> {
> static const char xml_header[] = "<?xml version=\"1.0\" encoding=\"utf-8\"?>";
> + HRESULT ret = E_FAIL;
> char *full_xml;
>
> /* TODO: Create SOAP envelope */
> @@ -46,11 +49,21 @@ static HRESULT write_and_send_message(IWSDiscoveryPublisherImpl *impl, WSD_SOAP_
> memcpy(full_xml, xml_header, sizeof(xml_header));
> full_xml[sizeof(xml_header)] = 0;
>
> - /* TODO: Send the message */
> + if (remote_address == NULL)
> + {
> + /* Send the message via UDP multicast */
> + if (send_udp_multicast(impl, full_xml, sizeof(xml_header), max_initial_delay))
> + ret = S_OK;
> + }
> + else
> + {
> + /* TODO: Send the message via UDP unicast */
> + FIXME("TODO: Send the message via UDP unicast\n");
> + }
>
> heap_free(full_xml);
>
> - return S_OK;
> + return ret;
> }
>
> HRESULT send_hello_message(IWSDiscoveryPublisherImpl *impl, LPCWSTR id, ULONGLONG metadata_ver, ULONGLONG instance_id,
> diff --git a/dlls/wsdapi/tests/discovery.c b/dlls/wsdapi/tests/discovery.c
> index e4a672b311..d81dcdc936 100644
> --- a/dlls/wsdapi/tests/discovery.c
> +++ b/dlls/wsdapi/tests/discovery.c
> @@ -570,7 +570,7 @@ static void Publish_tests(void)
>
> /* Publish the service */
> rc = IWSDiscoveryPublisher_Publish(publisher, publisherIdW, 1, 1, 1, NULL, NULL, NULL, NULL);
> - todo_wine ok(rc == S_OK, "Publish failed: %08x\n", rc);
> + ok(rc == S_OK, "Publish failed: %08x\n", rc);
>
> /* Wait up to 2 seconds for messages to be received */
> if (WaitForMultipleObjects(msgStorage->numThreadHandles, msgStorage->threadHandles, TRUE, 2000) == WAIT_TIMEOUT)
> @@ -583,7 +583,7 @@ static void Publish_tests(void)
> DeleteCriticalSection(&msgStorage->criticalSection);
>
> /* Verify we've received a message */
> - todo_wine ok(msgStorage->messageCount >= 1, "No messages received\n");
> + ok(msgStorage->messageCount >= 1, "No messages received\n");
>
> sprintf(endpointReferenceString, "<wsa:EndpointReference><wsa:Address>%s</wsa:Address></wsa:EndpointReference>", publisherId);
>
> diff --git a/dlls/wsdapi/wsdapi_internal.h b/dlls/wsdapi/wsdapi_internal.h
> index 631dfb3d7e..417eed9145 100644
> --- a/dlls/wsdapi/wsdapi_internal.h
> +++ b/dlls/wsdapi/wsdapi_internal.h
> @@ -49,6 +49,12 @@ typedef struct IWSDiscoveryPublisherImpl {
> BOOL publisherStarted;
> } IWSDiscoveryPublisherImpl;
>
> +/* network.c */
> +
> +BOOL init_networking(IWSDiscoveryPublisherImpl *impl);
> +void terminate_networking(IWSDiscoveryPublisherImpl *impl);
> +BOOL send_udp_multicast(IWSDiscoveryPublisherImpl *impl, char *data, int length, int max_initial_delay);
> +
> /* soap.c */
>
> HRESULT send_hello_message(IWSDiscoveryPublisherImpl *impl, LPCWSTR id, ULONGLONG metadata_ver, ULONGLONG instance_id,
>
>
More information about the wine-devel
mailing list