[PATCH] dpnet: Implement IDirectPlay8Client EnumHosts

Bruno Jesus 00cpxxx at gmail.com
Tue Nov 8 23:51:07 CST 2016


On Mon, Nov 7, 2016 at 6:24 AM, Alistair Leslie-Hughes
<leslie_alistair at hotmail.com> wrote:
> This patch allows the dpnet examples and various games to find the
> correct server running on the local network. (Server is either on
> windows or using native directplay).

Hi, thanks for the wonderful work. Can you share a few game titles
(better with demo available) that can be used to test?

> Feedback welcome. (Improvements, howto split the patch into smaller chunks).

Some ideas below, this is not really a review as I don't understand
some mechanics, it is more like a set of questions that may or may not
help you.

> Changes to CancelAsyncOperation have be obmitted, as it deserves a patch
> of it's own.  Currently no application tested seems to care if the operation
> isn't cancelled.
>
>
> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair at hotmail.com>
> ---
>  dlls/dpnet/client.c        | 399 ++++++++++++++++++++++++++++++++++++++++++++-
>  dlls/dpnet/dpnet_private.h |  25 +++
>  dlls/dpnet/dppacket.h      | 240 +++++++++++++++++++++++++++
>  dlls/dpnet/tests/client.c  |  81 ++++++++-
>  4 files changed, 738 insertions(+), 7 deletions(-)
>  create mode 100644 dlls/dpnet/dppacket.h
>
> diff --git a/dlls/dpnet/client.c b/dlls/dpnet/client.c
> index b92d940..1227be6 100644
> --- a/dlls/dpnet/client.c
> +++ b/dlls/dpnet/client.c
> @@ -29,10 +29,12 @@
>  #include "wingdi.h"
>  #include "winuser.h"
>  #include "objbase.h"
> +#include "ws2tcpip.h"
>  #include "wine/debug.h"
>
>  #include "dplay8.h"
>  #include "dpnet_private.h"
> +#include "dppacket.h"
>
>  WINE_DEFAULT_DEBUG_CHANNEL(dpnet);
>
> @@ -92,6 +94,21 @@ static ULONG WINAPI IDirectPlay8ClientImpl_Release(IDirectPlay8Client *iface)
>
>      if (!ref)
>      {
> +        struct async_tasks *cursor, *cursor2;
> +
> +        SetEvent(This->closeevent);
> +        closesocket(This->sock);
> +
> +        LIST_FOR_EACH_ENTRY_SAFE(cursor, cursor2, &This->tasks, struct async_tasks, entry)
> +        {
> +            IDirectPlay8Address_Release(cursor->device);
> +            list_remove(&cursor->entry);
> +            heap_free(cursor);
> +        }
> +
> +        CloseHandle(This->datathread);
> +        CloseHandle(This->closeevent);
> +
>          heap_free(This->username);
>          heap_free(This->data);
>          heap_free(This);
> @@ -130,6 +147,263 @@ static HRESULT WINAPI IDirectPlay8ClientImpl_EnumServiceProviders(IDirectPlay8Cl
>    return DPN_OK;
>  }
>
> +static SOCKET find_free_socket(void)

This function does more than finding a free socket, it also configures
it as expected by directplay, what about "setup_socket"?

> +{
> +    SOCKET sock;
> +    struct sockaddr_in addr;
> +    int value;
> +    int port = 2302;
> +    ULONG l = 1;

Where does this number 2302 comes from? A comment would be useful
here, the ULONG variable deserves a better name, what about discarding
the "value" variable and using only the ULONG in favor of it? There is
a mandatory cast in setsockopt so it wouldn't be a problem. As a style
choice I would group the int declarations in the same line.

> +    sock = socket(AF_INET, SOCK_DGRAM,  IPPROTO_UDP);
> +    if(sock == INVALID_SOCKET)
> +    {
> +        ERR("Cannot create socket.\n");

We all known this should never happen but since we have the ERR what
about adding the WSAGetLastError() to it?

> +        return sock;
> +    }
> +
> +    memset(&addr, 0, sizeof(addr));
> +    addr.sin_family = AF_INET;
> +    addr.sin_port = htons(port);
> +    addr.sin_addr.s_addr = htonl(INADDR_ANY);
> +
> +    value = 1;
> +    setsockopt(sock, SOL_SOCKET,  SO_BROADCAST, (char *)&value, sizeof(value));
> +    ioctlsocket(sock, FIONBIO, &l);

As commented above what about using the ULONG in both functions? Also,
why not do this only after a successful bind?

> +
> +    while(port < 2400)

Yet another magic number, just add a comment stating this is the final
checked port for directplay (if that is what this means).

> +    {
> +       if(!bind(sock, (struct sockaddr*)&addr, sizeof(addr)))
> +       {
> +           TRACE("bound to port (%d) successful.\n", port);
> +           return sock;
> +       }
> +       if(WSAGetLastError() == WSAEADDRINUSE)
> +           addr.sin_port = htons(++port);
> +       else
> +           break;
> +    }
> +
> +    ERR("No free ports available.\n");

The previous TRACE call was not capitalized, both ERR calls have the
first word capitalized, is it better to always start in uppercase or
lowercase? This applies to the rest of the TRACE calls that are mixing
upper/lowercase

> +    closesocket(sock);
> +    sock = INVALID_SOCKET;
> +
> +    return sock;

Maybe rewrite the socket() call check using a block or goto to use a
single return point in the function?

> +}
> +
> +struct dpthread_data
> +{
> +    SOCKET sock;
> +    DWORD bufsize;
> +
> +    HANDLE event;
> +    PFNDPNMESSAGEHANDLER msghandler;
> +
> +    struct list *tasks;
> +};
> +
> +static DWORD WINAPI data_thread(void *threaddata)
> +{
> +    struct dpthread_data *tdata = (struct dpthread_data *)threaddata;
> +    struct sockaddr_in addr;
> +    int addrlen;
> +    WORD payloadvalue = 0;
> +    char *buffer = heap_alloc(tdata->bufsize);

Is it work to check the allocation?

> +    int buflen;
> +    int received;
> +    struct async_tasks *cursor;
> +    struct ENUM_QUERY query;
> +
> +    /* This data doesn't change */
> +    query.lead = 0x00;
> +    query.command = PACKET_ENUM_QUERY;
> +    query.type = PACKET_QUERY_GUID;
> +
> +    TRACE("data thread start.\n");
> +
> +    while(TRUE)
> +    {
> +        if(list_count(tdata->tasks))
> +        {
> +            LIST_FOR_EACH_ENTRY(cursor, tdata->tasks, struct async_tasks, entry)
> +            {
> +                if(cursor->retrycnt && (GetTickCount() - cursor->starttime) >= cursor->retrytime)
> +                {
> +                    int err;
> +
> +                    TRACE("Sending Enum Query Request.\n");

Is it worth to also display the current retrycnt?

> +
> +                    query.payload = payloadvalue++;
> +                    query.application = cursor->app_guid;
> +
> +                    addr.sin_family = AF_INET;
> +                    addr.sin_port = htons(DPNA_DPNSVR_PORT);
> +                    /* TODO - Support a single host */
> +                    addr.sin_addr.s_addr = INADDR_BROADCAST;
> +
> +                    err = sendto(tdata->sock, (void *)&query, sizeof(query), 0,  (struct sockaddr *)&addr, sizeof(struct sockaddr_in));

Double space after "0, ".

> +                    if(err == -1)
> +                        ERR("sendto failed (%d).\n", WSAGetLastError());
> +                    else
> +                        TRACE("sent (%d).\n", err);

This TRACE is not necessary since we already have a trace before and
an ERR in case of problems.

> +
> +                    if(cursor->retrycnt != INFINITE)
> +                        cursor->retrycnt--;
> +
> +                    cursor->starttime = GetTickCount();
> +                }
> +            }
> +
> +            /* Query Enumeration Packets */
> +            buflen = tdata->bufsize;
> +            addrlen = sizeof(addr);
> +            received = recvfrom(tdata->sock, buffer, buflen, 0, (struct sockaddr *)&addr, &addrlen);
> +            if(received >= (int)sizeof(struct ENUM_HEADER))

There is no else to catch recvfrom errors, probably intentional since
this is an UDP socket, right?

> +            {
> +                struct ENUM_HEADER *header = (struct ENUM_HEADER *)buffer;
> +
> +                if(header->lead == 0)
> +                {
> +                    /* Enum Query */
> +                    switch(header->command)
> +                    {
> +                        case PACKET_ENUM_QUERY:
> +                        {
> +                            TRACE("Ignoring EnumQuery Packet.\n");
> +                            break;
> +                        }
> +                        /* Enum response */
> +                        case PACKET_ENUM_RESPONSE:
> +                        {
> +                            DPNMSG_ENUM_HOSTS_RESPONSE response;
> +                            DPN_APPLICATION_DESC desc;
> +                            struct ENUM_QUERY_RESPONSE *data = (struct ENUM_QUERY_RESPONSE *)header;
> +                            IDirectPlay8Address *sender = NULL;
> +                            char ip[INET6_ADDRSTRLEN] = {0};

Is it possible to be really an IPv6?

> +                            int port;
> +                            HRESULT ret;
> +
> +                            TRACE("PACKET_ENUM_RESPONSE.\n");
> +
> +                            if(received <= sizeof(struct ENUM_QUERY_RESPONSE))
> +                            {
> +                                ERR("May not have received enough data.\n");
> +                                SetEvent(tdata->event);
> +                                break;

Is the <= intentional? In my imagination it would be correct to
receive == sizeof(struct). This should only happen due to game bugs,
right?

> +                            }
> +
> +                            memset(&response, 0, sizeof(DPNMSG_ENUM_HOSTS_RESPONSE));
> +                            memset(&desc, 0, sizeof(DPN_APPLICATION_DESC));
> +
> +                            response.dwSize = sizeof(DPNMSG_ENUM_HOSTS_RESPONSE);
> +
> +                            ret = DPN_OK;
> +
> +                            LIST_FOR_EACH_ENTRY(cursor, tdata->tasks, struct async_tasks, entry)

This is the very same loop in the beginning of the function, what are
these tasks and why do we need to run through all of them while inside
the outer loop on the same struct?

> +                            {
> +                                if(IsEqualGUID(&cursor->app_guid, &data->application) ||
> +                                   IsEqualGUID(&cursor->app_guid, &IID_NULL))             /* all applications */
> +                                {

Just a style preference, why not invert the if condition to reduce the
code indentation?

> +                                    /*
> +                                     *  We need to create an Address Object with the Port and IP address.
> +                                     *  Applications are required to AddRef this object if they want to keep a reference.
> +                                     */
> +                                    ret = DPNET_CreateDirectPlay8Address(NULL, NULL, &IID_IDirectPlay8Address, (void**)&sender);
> +                                    if(FAILED(ret))
> +                                        break;
> +
> +                                    inet_ntop(addr.sin_family, &addr.sin_addr, ip, sizeof(ip));
> +                                    port = ntohs(addr.sin_port);
> +                                    TRACE("Server IP address %s:%d\n", ip, port);
> +
> +                                    if(FAILED(IDirectPlay8Address_AddComponent(sender, DPNA_KEY_HOSTNAME, &ip, strlen(ip)+1, DPNA_DATATYPE_STRING_ANSI)))
> +                                    {
> +                                        ERR("Failed to add DPNA_KEY_HOSTNAME\n");
> +                                        IDirectPlay8Address_Release(sender);
> +                                        SetEvent(tdata->event);
> +                                        break;

If CreateDirectPlay8Address fails you are not calling SetEvent, is
that intentional? Because if IDirectPlay8Address_AddComponent fails
you call SetEvent.

> +                                    }
> +                                    if(FAILED(IDirectPlay8Address_AddComponent(sender, DPNA_KEY_PORT, &port, sizeof(DWORD), DPNA_DATATYPE_DWORD)))
> +                                    {
> +                                        ERR("Failed to add DPNA_KEY_PORT\n");
> +                                        IDirectPlay8Address_Release(sender);
> +                                        SetEvent(tdata->event);
> +                                        break;
> +                                    }
> +
> +                                    response.pAddressSender = sender;
> +                                    response.pApplicationDescription = &desc;
> +                                    response.pAddressDevice = cursor->device;
> +
> +                                    desc.guidInstance     = data->instance;
> +                                    desc.guidApplication  = data->application;
> +                                    desc.dwCurrentPlayers = data->current_players;
> +                                    desc.dwMaxPlayers     = data->max_players;
> +
> +                                    if(data->session_offset && data->session_offset < received)
> +                                        desc.pwszSessionName  = (WCHAR*)(((char*)data)+data->session_offset+sizeof(struct ENUM_HEADER));
> +
> +                                    if(tdata->msghandler)
> +                                        ret = tdata->msghandler(NULL, DPN_MSGID_ENUM_HOSTS_RESPONSE, &response);

If all the above was only made if a msghandler is present can you
simply ignore all data received when msghandler is not present?

> +
> +                                    IDirectPlay8Address_Release(sender);
> +
> +                                    if(cursor->retrycnt != INFINITE)
> +                                        cursor->retrycnt = 0;
> +
> +                                    /* If a userhandler function returns anything other than DPN_OK we need to stop. */
> +                                    if(ret != DPN_OK)
> +                                        break;
> +                                }
> +                            }
> +
> +                            if(ret != DPN_OK)
> +                                SetEvent(tdata->event);

Why can't you SetEvent inside the previous ret != DPN_OK since it is
breaking the inner loop anyway?

> +
> +                            break;
> +                        }
> +                        default:
> +                        {
> +                            FIXME("Unsupported query command 0x%08x.\n", header->command);
> +                        }
> +                    }
> +                }
> +                else
> +                {
> +                    struct DFRAME_PACKET *dframe = (struct DFRAME_PACKET *)buffer;
> +                    FIXME("DFRAME received - command 0x%08x, control 0x%08x.\n", dframe->command, dframe->control);
> +                }
> +            }
> +
> +            LIST_FOR_EACH_ENTRY(cursor, tdata->tasks, struct async_tasks, entry)
> +            {
> +                if(!cursor->retrycnt && (GetTickCount() - cursor->starttime) >= cursor->timeout)
> +                {
> +                    list_remove( &cursor->entry );
> +                    IDirectPlay8Address_Release(cursor->device);
> +                    if(cursor->aysncmutex)
> +                    {
> +                        TRACE("Removing async event\n");
> +                        SetEvent(cursor->aysncmutex);
> +                    }
> +                    /* TODO - Send DPN_MSGID_ASYNC_OP_COMPLETE if not ASYNC */
> +                    heap_free(cursor);
> +                }
> +            }
> +        }
> +
> +        if(WaitForSingleObject(tdata->event, 500) != WAIT_TIMEOUT)
> +        {
> +            TRACE("Event Signaled.\n");
> +            break;
> +        }
> +    }
> +
> +    heap_free(buffer);
> +
> +    return 0;
> +}
> +
>  static HRESULT WINAPI IDirectPlay8ClientImpl_EnumHosts(IDirectPlay8Client *iface,
>          PDPN_APPLICATION_DESC const pApplicationDesc, IDirectPlay8Address * const pAddrHost,
>          IDirectPlay8Address * const pDeviceInfo, void * const pUserEnumData,
> @@ -137,9 +411,111 @@ static HRESULT WINAPI IDirectPlay8ClientImpl_EnumHosts(IDirectPlay8Client *iface
>          const DWORD dwTimeOut, void * const pvUserContext, DPNHANDLE * const pAsyncHandle,
>          const DWORD dwFlags)
>  {
> -  IDirectPlay8ClientImpl *This = impl_from_IDirectPlay8Client(iface);
> -  FIXME("(%p):(%p,%p,%x): Stub\n", This, pvUserContext, pAsyncHandle, dwFlags);
> -  return DPN_OK;
> +    IDirectPlay8ClientImpl *This = impl_from_IDirectPlay8Client(iface);
> +    HRESULT hr;
> +    HANDLE async_mutex = NULL;
> +    struct async_tasks *task;
> +
> +    TRACE("(%p):(%p,%p,%p,%p,%u,%u,%u,%u,%p,%p,%x)\n", This, pApplicationDesc, pAddrHost, pDeviceInfo, pUserEnumData,
> +        dwUserEnumDataSize, dwEnumCount, dwRetryInterval, dwTimeOut, pvUserContext, pAsyncHandle, dwFlags);
> +
> +    if(!This->msghandler)
> +        return DPNERR_UNINITIALIZED;
> +
> +    if(dwFlags & DPNENUMHOSTS_SYNC)
> +    {
> +        if(pAsyncHandle)
> +            return DPNERR_INVALIDPARAM;
> +
> +        FIXME("Flag DPNENUMHOSTS_SYNC currently not supported (%u, %u, %u).\n", dwEnumCount, dwRetryInterval, dwTimeOut);
> +    }
> +
> +    if(dwUserEnumDataSize > This->spcaps.dwMaxEnumPayloadSize)
> +        return DPNERR_ENUMQUERYTOOLARGE;
> +
> +    if(This->sock == INVALID_SOCKET)
> +    {
> +        This->sock = find_free_socket();
> +        if(This->sock == INVALID_SOCKET)
> +            return DPNERR_USERCANCEL; /* Pretend the user cancalled this operation. */
> +    }
> +
> +    if(!This->closeevent)
> +    {
> +        This->closeevent = CreateEventA( NULL, TRUE, FALSE, NULL);
> +        if(!This->closeevent)
> +        {
> +            WARN("Failed to create Event\n");
> +            closesocket(This->sock);
> +            This->sock = INVALID_SOCKET;
> +            return DPNERR_GENERIC;
> +        }
> +    }
> +
> +    task = heap_alloc(sizeof(struct async_tasks));
> +    if(!task)
> +    {
> +        closesocket(This->sock);
> +        This->sock = INVALID_SOCKET;
> +        return E_OUTOFMEMORY;

Shoud you destroy the closeevent too?

> +    }
> +
> +    task->handle = PtrToUlong(task);
> +    task->type = enumhost;
> +    task->app_guid = pApplicationDesc->guidApplication;
> +    task->starttime = 0;
> +    task->retrycnt   = !dwEnumCount ? This->spcaps.dwDefaultEnumCount : dwEnumCount;
> +    task->retrytime  = dwRetryInterval == INFINITE ? This->spcaps.dwDefaultEnumRetryInterval : dwRetryInterval;
> +    task->timeout    = dwTimeOut == 0 ? This->spcaps.dwDefaultEnumTimeout : dwTimeOut;

!dwEnumCount but dwTimeout == 0, style inconsistency?

> +    if(dwFlags & DPNENUMHOSTS_SYNC)
> +       async_mutex = CreateMutexA(NULL, FALSE, NULL);
> +
> +    task->aysncmutex = async_mutex;
> +    hr = IDirectPlay8Address_Duplicate(pDeviceInfo, &task->device);
> +    if(FAILED(hr))
> +    {
> +        ERR("Failed to duplicate Device address (0x%08x).\n", hr);
> +
> +        closesocket(This->sock);
> +        This->sock = INVALID_SOCKET;
> +        heap_free(task);
> +
> +        return E_OUTOFMEMORY;

Free closeevent? Free async_mutex?

> +    }
> +
> +    list_add_tail(&This->tasks, &task->entry);
> +
> +    if(!This->datathread)
> +    {
> +        struct dpthread_data *tdata = heap_alloc(sizeof(struct dpthread_data));
> +        if(!tdata)
> +        {
> +            closesocket(This->sock);
> +            This->sock = INVALID_SOCKET;
> +
> +            list_remove( &task->entry );
> +            IDirectPlay8Address_Release(task->device);
> +            heap_free(task);
> +
> +            return E_OUTOFMEMORY;
> +        }
> +
> +        tdata->sock       = This->sock;
> +        tdata->bufsize    = This->spcaps.dwSystemBufferSize;
> +        tdata->msghandler = This->msghandler;
> +        tdata->event      = This->closeevent;
> +        tdata->tasks      = &This->tasks;
> +
> +        This->datathread = CreateThread(NULL, 0, &data_thread, tdata, 0, 0);
> +    }
> +
> +    if(async_mutex)
> +        WaitForSingleObject(async_mutex,INFINITE);
> +
> +    if(pAsyncHandle)
> +        *pAsyncHandle = task->handle;
> +
> +    return (dwFlags & DPNENUMHOSTS_SYNC) ? DPN_OK : DPNSUCCESS_PENDING;
>  }
>
>  static HRESULT WINAPI IDirectPlay8ClientImpl_CancelAsyncOperation(IDirectPlay8Client *iface,
> @@ -256,8 +632,23 @@ static HRESULT WINAPI IDirectPlay8ClientImpl_GetServerAddress(IDirectPlay8Client
>  static HRESULT WINAPI IDirectPlay8ClientImpl_Close(IDirectPlay8Client *iface, const DWORD dwFlags)
>  {
>      IDirectPlay8ClientImpl *This = impl_from_IDirectPlay8Client(iface);
> +    struct async_tasks *cursor, *cursor2;
>      FIXME("(%p):(%x): Stub\n", This, dwFlags);
>
> +    SetEvent(This->closeevent);
> +
> +    LIST_FOR_EACH_ENTRY_SAFE(cursor, cursor2, &This->tasks, struct async_tasks, entry)
> +    {
> +        IDirectPlay8Address_Release(cursor->device);
> +        list_remove(&cursor->entry);
> +        heap_free(cursor);
> +    }
> +
> +    CloseHandle(This->datathread);
> +    This->datathread = NULL;
> +    CloseHandle(This->closeevent);
> +    This->closeevent = NULL;
> +
>      This->msghandler = NULL;
>
>      return DPN_OK;
> @@ -376,6 +767,8 @@ HRESULT DPNET_CreateDirectPlay8Client(IClassFactory *iface, IUnknown *pUnkOuter,
>
>      client->IDirectPlay8Client_iface.lpVtbl = &DirectPlay8Client_Vtbl;
>      client->ref = 1;
> +    client->sock = INVALID_SOCKET;
> +    list_init(&client->tasks);
>
>      init_dpn_sp_caps(&client->spcaps);
>
> diff --git a/dlls/dpnet/dpnet_private.h b/dlls/dpnet/dpnet_private.h
> index 89e0777..763b1c2 100644
> --- a/dlls/dpnet/dpnet_private.h
> +++ b/dlls/dpnet/dpnet_private.h
> @@ -41,6 +41,26 @@ typedef struct IDirectPlay8AddressImpl IDirectPlay8AddressImpl;
>  typedef struct IDirectPlay8LobbiedApplicationImpl IDirectPlay8LobbiedApplicationImpl;
>  typedef struct IDirectPlay8ThreadPoolImpl IDirectPlay8ThreadPoolImpl;
>
> +enum async_type { enumhost, connection, sendmsg };
> +
> +struct async_tasks
> +{
> +    DPNHANDLE handle;
> +    BYTE type;
> +
> +    IDirectPlay8Address *device;
> +
> +    PFNDPNMESSAGEHANDLER msghandler;
> +    GUID app_guid;
> +    DWORD starttime;
> +    DWORD retrycnt;
> +    DWORD retrytime;
> +    DWORD timeout;
> +    HANDLE aysncmutex;
> +
> +    struct list entry;
> +};
> +
>  /* ------------------ */
>  /* IDirectPlay8Client */
>  /* ------------------ */
> @@ -62,6 +82,11 @@ struct IDirectPlay8ClientImpl
>      DWORD datasize;
>
>      DPN_SP_CAPS spcaps;
> +
> +    SOCKET sock;
> +    HANDLE closeevent;
> +    HANDLE datathread;
> +    struct list tasks;
>  };
>
>  /* ------------------- */
> diff --git a/dlls/dpnet/dppacket.h b/dlls/dpnet/dppacket.h
> new file mode 100644
> index 0000000..297640a
> --- /dev/null
> +++ b/dlls/dpnet/dppacket.h
> @@ -0,0 +1,240 @@
> +/*
> + * dpnet packet structures
> + *
> + * Copyright 2016 Alistair Leslie-Hughes
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
> + */
> +#ifndef __WINE_DPNET_PACKET_H
> +#define __WINE_DPNET_PACKET_H
> +
> +#include <pshpack1.h>
> +
> +struct ENUM_HEADER
> +{
> +    BYTE lead;
> +    BYTE command;
> +    WORD payload;
> +};
> +
> +struct ENUM_QUERY
> +{
> +    BYTE lead;
> +    BYTE command;
> +    WORD payload;
> +    BYTE type;
> +    GUID application;
> +};
> +
> +struct ENUM_QUERY_RESPONSE
> +{
> +    BYTE   lead;
> +    BYTE   command;
> +    WORD   payload;
> +    DWORD  reply_offset;
> +    DWORD  response_size;
> +    DWORD  desc_size;
> +    DWORD  desc_flags;
> +    DWORD  max_players;
> +    DWORD  current_players;
> +    DWORD  session_offset;
> +    DWORD  session_size;
> +    DWORD  Password_offset;
> +    DWORD  Password_size;
> +    DWORD  reserved_offset;
> +    DWORD  reserved_size;
> +    DWORD  application_offset;
> +    DWORD  application_size;
> +    GUID   instance;
> +    GUID   application;
> +};

This file seems to be private, so can we have all struct attributes
starting in lowercase?

> +
> +struct DFRAME_PACKET
> +{
> +    BYTE command;
> +    BYTE control;
> +    BYTE seq;
> +    BYTE nseq;
> +};
> +
> +/* 2.2.1.8 DN_ACK_CONNECT_INFO */
> +struct DN_ACK_CONNECT_INFO
> +{
> +    DWORD type;
> +};
> +
> +struct DFRAME_PACKET_KEEPALIVE
> +{
> +    BYTE command;
> +    BYTE control;
> +    BYTE seq;
> +    BYTE nseq;
> +    DWORD session;
> +};
> +
> +struct DN_MESSAGE_CONNECT
> +{
> +    BYTE command;
> +    BYTE opcode;
> +    BYTE msgid;
> +    BYTE rspid;
> +    DWORD protocol;
> +    DWORD session;
> +    DWORD timestamp;
> +};
> +
> +
> +struct SACK_PACKET
> +{
> +    BYTE command;
> +    BYTE opcode;
> +    BYTE flags;
> +    BYTE retry;
> +    BYTE nseq;
> +    BYTE nrcv;
> +    WORD padding;
> +    DWORD timestamp;
> +};
> +
> +/* 2.2.1.1 DN_INTERNAL_MESSAGE_PLAYER_CONNECT_INFO */
> +struct DN_INTERNAL_MESSAGE_PLAYER_CONNECT_INFO
> +{
> +    DWORD type;
> +    DWORD flags;
> +    DWORD version;
> +    DWORD name_offset;
> +    DWORD name_size;
> +    DWORD data_offset;
> +    DWORD data_size;
> +    DWORD pwd_offset;
> +    DWORD pwd_size;
> +    DWORD connect_offset;
> +    DWORD connect_size;
> +    DWORD url_offset;
> +    DWORD url_size;
> +    GUID  instance;
> +    GUID  application;
> +};
> +
> +struct DN_INTERNAL_MESSAGE_PLAYER_CONNECT_INFO_EX
> +{
> +    DWORD type;
> +    DWORD flags;
> +    DWORD version;
> +    DWORD name_offset;
> +    DWORD name_size;
> +    DWORD data_offset;
> +    DWORD data_size;
> +    DWORD pwd_offset;
> +    DWORD pwd_size;
> +    DWORD connect_offset;
> +    DWORD connect_size;
> +    DWORD url_offset;
> +    DWORD url_size;
> +    GUID  instance;
> +    GUID  application;
> +    DWORD alt_address_offset;
> +    DWORD alt_address_size;
> +};
> +
> +/* 2.2.1.4 DN_SEND_CONNECT_INFO */
> +struct DN_SEND_CONNECT_INFO
> +{
> +    DWORD type;                   /* DN_MSG_INTERNAL_SEND_CONNECT_INFO */
> +    DWORD reply_offset;
> +    DWORD reply_size;
> +    DWORD size;
> +    DWORD flags;
> +    DWORD max_players;
> +    DWORD cur_players;
> +    DWORD session_offset;
> +    DWORD session_size;
> +    DWORD password_offset;
> +    DWORD password_size;
> +    DWORD reserved_offset;
> +    DWORD reserved_size;
> +    DWORD appdata_offset;
> +    DWORD appdata_size;
> +    GUID  instance;
> +    GUID  application;
> +    DWORD dpnid;
> +    DWORD version;
> +    DWORD ver_not_used;
> +    DWORD entry_count;
> +    DWORD membership_count;
> +};
> +
> +#define PACKET_ENUM_QUERY                     0x02
> +#define PACKET_ENUM_RESPONSE                  0x03
> +
> +#define PACKET_QUERY_GUID                     0x01
> +#define PACKET_QUERY_NO_GUID                  0x02
> +
> +#define APPLICATIONDESCSIZE                   0x00000050
> +
> +#define PACKET_COMMAND_DATA                   0x01
> +#define PACKET_COMMAND_RELIABLE               0x02
> +#define PACKET_COMMAND_SEQUENTIAL             0x04
> +#define PACKET_COMMAND_POLL                   0x08
> +#define PACKET_COMMAND_NEW_MSG                0x10
> +#define PACKET_COMMAND_END_MSG                0x20
> +#define PACKET_COMMAND_USER_1                 0x40
> +#define PACKET_COMMAND_USER_2                 0x80
> +#define PACKET_COMMAND_CFRAME                 PACKET_COMMAND_USER_2
> +
> +#define PACKET_CONTROL_RETRY                  0x01
> +#define PACKET_CONTROL_KEEPALIVE_OR_CORRELATE 0x02
> +#define PACKET_CONTROL_COALESCE               0x04
> +#define PACKET_CONTROL_END_STREAM             0x08
> +#define PACKET_CONTROL_SACK1                  0x10
> +#define PACKET_CONTROL_SACK2                  0x20
> +#define PACKET_CONTROL_SEND1                  0x40
> +#define PACKET_CONTROL_SEND2                  0x80
> +
> +#define PROTOCOL_VERSION                      0x00010006
> +
> +#define FRAME_EXOPCODE_CONNECT                0x01
> +#define FRAME_EXOPCODE_CONNECTED              0x02
> +#define FRAME_EXOPCODE_CONNECTED_SIGNED       0x03
> +#define FRAME_EXOPCODE_HARD_DISCONNECT        0x04
> +#define FRAME_EXOPCODE_SACK                   0x06
> +
> +#define PACKET_SIGNING_FAST                   0x00000001
> +#define PACKET_SIGNING_FULL                   0x00000002
> +
> +#define SACK_FLAGS_RESPONSE                   0x01
> +#define SACK_FLAGS_SACK_MASK1                 0x02
> +#define SACK_FLAGS_SACK_MASK2                 0x04
> +#define SACK_FLAGS_SEND_MASK1                 0x08
> +#define SACK_FLAGS_SEND_MASK2                 0x10
> +
> +#define DN_MSG_INTERNAL_PLAYER_CONNECT_INFO   0x000000C1
> +#define DN_MSG_INTERNAL_SEND_CONNECT_INFO     0x000000C2
> +#define DN_MSG_INTERNAL_ACK_CONNECT_INFO      0x000000C3
> +
> +#define DP_OBECT_TYPE_CLIENT                  0x00000002
> +#define DP_OBECT_TYPE_PEER                    0x00000004
> +
> +#define DP_OBECT_NET_VER_8_0                  0x00000001
> +#define DP_OBECT_NET_VER_8_1                  0x00000002
> +#define DP_OBECT_NET_VER_POCKETPC             0x00000003
> +#define DP_OBECT_NET_VER_W2K3                 0x00000005
> +#define DP_OBECT_NET_VER_8_2                  0x00000006
> +#define DP_OBECT_NET_VER_9_0                  0x00000007
> +#define DP_OBECT_NET_VER_9_0A                 0x00000008

This file seems to be adding way more stuff than you are using
currently, is it better to add as things are developed?

> +#include "poppack.h"
> +
> +#endif
> diff --git a/dlls/dpnet/tests/client.c b/dlls/dpnet/tests/client.c
> index 4753d74..608f0e5 100644
> --- a/dlls/dpnet/tests/client.c
> +++ b/dlls/dpnet/tests/client.c
> @@ -29,9 +29,29 @@ static IDirectPlay8Client* client = NULL;
>  static IDirectPlay8LobbiedApplication* lobbied = NULL;
>  static const GUID appguid = { 0xcd0c3d4b, 0xe15e, 0x4cf2, { 0x9e, 0xa8, 0x6e, 0x1d, 0x65, 0x48, 0xc5, 0xa5 } };
>
> +static HRESULT   lastAsyncCode   = E_FAIL;
> +static DPNHANDLE lastAsyncHandle = 0;
> +
> +#define CHECK_LAST_ASYNC_OP(hr, handle) \
> +        ok(lastAsyncCode == hr, "got 0x%08x\n", lastAsyncCode); \
> +        ok(lastAsyncHandle == handle, "got 0x%08x\n", handle); \
> +        lastAsyncCode = E_FAIL; lastAsyncHandle = 0;
> +
>  static HRESULT WINAPI DirectPlayMessageHandler(PVOID context, DWORD message_id, PVOID buffer)
>  {
> -    trace("DirectPlayMessageHandler: 0x%08x\n", message_id);
> +    switch(message_id)
> +    {
> +        case DPN_MSGID_ASYNC_OP_COMPLETE:
> +        {
> +            PDPNMSG_ASYNC_OP_COMPLETE async_msg = (PDPNMSG_ASYNC_OP_COMPLETE)buffer;
> +            lastAsyncCode   = async_msg->hResultCode;
> +            lastAsyncHandle = async_msg->hAsyncOp;
> +            break;
> +        }
> +        default:
> +                trace("DirectPlayMessageHandler: 0x%08x\n", message_id);
> +
> +    }
>      return S_OK;
>  }
>
> @@ -144,16 +164,22 @@ static void test_enum_service_providers(void)
>  static void test_enum_hosts(void)
>  {
>      HRESULT hr;
> +    IDirectPlay8Client *client2;
>      IDirectPlay8Address *host = NULL;
>      IDirectPlay8Address *local = NULL;
>      DPN_APPLICATION_DESC appdesc;
> -    DPNHANDLE async = 0;
> +    DPNHANDLE async = 0, async2 = 0;
>      static const WCHAR localhost[] = {'1','2','7','.','0','.','0','.','1',0};
> +    DPN_SP_CAPS caps;
> +    char *data;
>
>      memset( &appdesc, 0, sizeof(DPN_APPLICATION_DESC) );
>      appdesc.dwSize  = sizeof( DPN_APPLICATION_DESC );
>      appdesc.guidApplication  = appguid;
>
> +    hr = CoCreateInstance(&CLSID_DirectPlay8Client, NULL, CLSCTX_INPROC_SERVER, &IID_IDirectPlay8Client, (void **)&client2);
> +    ok(hr == S_OK, "CoCreateInstance failed with 0x%x\n", hr);
> +
>      hr = CoCreateInstance( &CLSID_DirectPlay8Address, NULL, CLSCTX_ALL, &IID_IDirectPlay8Address, (LPVOID*)&local);
>      ok(hr == S_OK, "IDirectPlay8Address failed with 0x%08x\n", hr);
>
> @@ -170,16 +196,63 @@ static void test_enum_hosts(void)
>                                                           DPNA_DATATYPE_STRING);
>      ok(hr == S_OK, "IDirectPlay8Address failed with 0x%08x\n", hr);
>
> +    caps.dwSize = sizeof(DPN_SP_CAPS);
> +
> +    hr = IDirectPlay8Client_GetSPCaps(client, &CLSID_DP8SP_TCPIP, &caps, 0);
> +    ok(hr == DPN_OK, "got %x\n", hr);
> +    data = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, caps.dwMaxEnumPayloadSize + 1);
> +
> +    hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, NULL, 0, 2, 1000, 1000, NULL,  &async, DPNENUMHOSTS_SYNC);
> +    ok(hr == DPNERR_INVALIDPARAM, "got 0x%08x\n", hr);
> +
> +    hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, data, caps.dwMaxEnumPayloadSize + 1,
> +                                        INFINITE, 0, INFINITE, NULL,  &async, DPNENUMHOSTS_SYNC);
> +    ok(hr == DPNERR_INVALIDPARAM, "got 0x%08x\n", hr);
> +
> +    async = 0;
> +    hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, data, caps.dwMaxEnumPayloadSize + 1, INFINITE, 0,
> +                                        INFINITE, NULL,  &async, 0);
> +    ok(hr == DPNERR_ENUMQUERYTOOLARGE, "got 0x%08x\n", hr);
> +    ok(!async, "Handle returned\n");
> +
> +    async = 0;
> +    hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, data, caps.dwMaxEnumPayloadSize, INFINITE, 0, INFINITE,
> +                                        NULL,  &async, 0);
> +    ok(hr == DPNSUCCESS_PENDING, "got 0x%08x\n", hr);
> +    ok(async, "No Handle returned\n");
> +
> +    /* This CancelAsyncOperation doesn't generate a DPN_MSGID_ASYNC_OP_COMPLETE */
> +    hr = IDirectPlay8Client_CancelAsyncOperation(client, async, 0);
> +    ok(hr == S_OK, "got 0x%08x\n", hr);
> +    HeapFree(GetProcessHeap(), 0, data);
> +
> +    /* No Initialize has been called on client2. */
> +    hr = IDirectPlay8Client_EnumHosts(client2, &appdesc, host, local, NULL, 0, INFINITE, 0, INFINITE, NULL,  &async, 0);
> +    ok(hr == DPNERR_UNINITIALIZED, "IDirectPlay8Client_EnumHosts failed with 0x%08x\n", hr);
> +
>      /* Since we are running asynchronously, EnumHosts returns DPNSUCCESS_PENDING. */
>      hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, NULL, 0, INFINITE, 0, INFINITE, NULL,  &async, 0);
> -    todo_wine ok(hr == DPNSUCCESS_PENDING, "IDirectPlay8Client_EnumServiceProviders failed with 0x%08x\n", hr);
> -    todo_wine ok(async, "No Handle returned\n");
> +    ok(hr == DPNSUCCESS_PENDING, "IDirectPlay8Client_EnumHosts failed with 0x%08x\n", hr);
> +    ok(async, "No Handle returned\n");
> +
> +    hr = IDirectPlay8Client_EnumHosts(client, &appdesc, host, local, NULL, 0, INFINITE, 0, INFINITE, NULL,  &async2, 0);
> +    ok(hr == DPNSUCCESS_PENDING, "IDirectPlay8Client_EnumHosts failed with 0x%08x\n", hr);
> +    ok(async2, "No Handle returned\n");
> +    ok(async2 != async, "Same handle returned.\n");
> +
> +    Sleep(500);
>
>      hr = IDirectPlay8Client_CancelAsyncOperation(client, async, 0);
>      ok(hr == S_OK, "IDirectPlay8Client_CancelAsyncOperation failed with 0x%08x\n", hr);
> +    todo_wine { CHECK_LAST_ASYNC_OP(DPNERR_USERCANCEL, async); }
> +
> +    hr = IDirectPlay8Client_CancelAsyncOperation(client, async2, 0);
> +    ok(hr == S_OK, "IDirectPlay8Client_CancelAsyncOperation failed with 0x%08x\n", hr);
> +    todo_wine { CHECK_LAST_ASYNC_OP(DPNERR_USERCANCEL, async2); }
>
>      IDirectPlay8Address_Release(local);
>      IDirectPlay8Address_Release(host);
> +    IDirectPlay8Client_Release(client2);
>  }
>
>  static void test_get_sp_caps(void)
> --
> 1.9.1
>
>
>



More information about the wine-devel mailing list