[PATCH] iphlpapi: call WSASetLastError in NotifyAddrChange/NotifyRouteChange

[Sebastian Lackner]

Answering here to the mail that only was send to wine-devel. If the patch for wine-patches was updated, please ignore any feedback which isn't valid anymore.

On 03.11.2014 08:27, Hao Peng wrote:
> diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c
> index 4d66810..84c58af 100644
> --- a/dlls/iphlpapi/iphlpapi_main.c
> +++ b/dlls/iphlpapi/iphlpapi_main.c
> @@ -64,6 +64,21 @@ WINE_DEFAULT_DEBUG_CHANNEL(iphlpapi);
>  #define INADDR_NONE ~0UL
>  #endif
>  
> +typedef void (WINAPI *LPFN_WSASETLASTERROR)(int);// from winsock2.h

You shouldn't use C++ style comments here. Moreover you should probably include the header file, instead of copying the definition.

> +static LPFN_WSASETLASTERROR pWSASetLastError = NULL;
> +
> +BOOL LoadWinsock(void)

Should be static, and named with lower-case letters, since this is just an internal helper function.

> +{
> +  HANDLE mod = NULL;
> +  if (pWSASetLastError != NULL) return TRUE;
> +  mod = LoadLibraryA("ws2_32.dll");
> +  if (NULL != mod)
> +  {
> +    pWSASetLastError = (LPFN_WSASETLASTERROR)GetProcAddress(mod, "WSASetLastError");
> +  }
> +  return (NULL != pWSASetLastError);
> +}

This is a bit ugly because you can't free the library anymore, and the refcount increases each time when ws2_32.dll is present but doesn't include WSASetLastError.

> +
>  /******************************************************************
>   *    AddIPAddress (IPHLPAPI.@)
>   *
> @@ -145,7 +160,7 @@ static int IpAddrTableSorter(const void *a, const void *b)
>  /******************************************************************
>   *    AllocateAndGetIpAddrTableFromStack (IPHLPAPI.@)
>   *
> - * Get interface-to-IP address mapping table. 
> + * Get interface-to-IP address mapping table.

You should avoid reformatting all the lines. Those are unrelated to the patch. Only change whitespace issues if they are close to other parts of the code you touch. Same also below.

>   * Like GetIpAddrTable(), but allocate the returned table from heap.
>   *
>   * PARAMS
> @@ -251,7 +266,7 @@ DWORD WINAPI CreateIpNetEntry(PMIB_IPNETROW pArpEntry)
>   * Create a Proxy ARP (PARP) entry for an IP address.

[...]

>              *pOutBufLen = size;
>              ret = ERROR_INSUFFICIENT_BUFFER;
> @@ -1253,7 +1268,7 @@ ULONG WINAPI DECLSPEC_HOTPATCH GetAdaptersAddresses(ULONG family, ULONG flags, P
>      InterfaceIndexTable *table;
>      ULONG i, size, dns_server_size = 0, dns_suffix_size, total_size, ret = ERROR_NO_DATA;
>  
> -    TRACE("(%d, %08x, %p, %p, %p)\n", family, flags, reserved, aa, buflen);
> +    TRACE("(%d, %08x, %p, %p, %p, %d)\n", family, flags, reserved, aa, buflen, *buflen);

That looks also a bit unrelated?

>  
>      if (!buflen) return ERROR_INVALID_PARAMETER;
>  
> @@ -1687,7 +1702,7 @@ DWORD WINAPI GetInterfaceInfo(PIP_INTERFACE_INFO pIfTable, PULONG dwOutBufLen)
>  /******************************************************************
>   *    GetIpAddrTable (IPHLPAPI.@)
>   *

[...]

>   *  If bOrder is true, the returned table will be sorted, first by
> @@ -2276,9 +2291,16 @@ DWORD WINAPI IpRenewAddress(PIP_ADAPTER_INDEX_MAP AdapterInfo)
>   */
>  DWORD WINAPI NotifyAddrChange(PHANDLE Handle, LPOVERLAPPED overlapped)
>  {
> +  if (!overlapped)
> +  {
> +    FIXME("Synchronous method not supported\n");
> +    return ERROR_NOT_SUPPORTED;
> +  }
>    FIXME("(Handle %p, overlapped %p): stub\n", Handle, overlapped);
>    if (Handle) *Handle = INVALID_HANDLE_VALUE;
> -  if (overlapped) ((IO_STATUS_BLOCK *) overlapped)->u.Status = STATUS_PENDING;
> +  ((IO_STATUS_BLOCK *) overlapped)->u.Status = STATUS_PENDING;
> +  if (LoadWinsock())
> +    pWSASetLastError(WSA_IO_PENDING);

This probably needs a test to ensure it matches the native version, if we don't have any yet.

>    return ERROR_IO_PENDING;
>  }
>  
> @@ -2301,8 +2323,19 @@ DWORD WINAPI NotifyAddrChange(PHANDLE Handle, LPOVERLAPPED overlapped)
>   */
>  DWORD WINAPI NotifyRouteChange(PHANDLE Handle, LPOVERLAPPED overlapped)
>  {
> +  if (!overlapped)
> +  {
> +    FIXME("Synchronous method not supported\n");
> +    return ERROR_NOT_SUPPORTED;
> +  }
>    FIXME("(Handle %p, overlapped %p): stub\n", Handle, overlapped);
> -  return ERROR_NOT_SUPPORTED;
> +  if (Handle) *Handle = INVALID_HANDLE_VALUE;
> +  ((IO_STATUS_BLOCK *) overlapped)->u.Status = STATUS_PENDING;
> +  if (LoadWinsock())
> +  {
> +    pWSASetLastError(WSA_IO_PENDING);
> +  }

Same here. Why different coding style compared to above, although the code is the same?

> +  return ERROR_IO_PENDING;
>  }
>