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:EndpointReferencewsa: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,