[PATCH 1/3] dpwsockx: Stub implementation

Ismael Barros² razielmine at gmail.com
Sat Aug 29 06:00:53 CDT 2009


On Sat, Aug 29, 2009 at 12:25 PM, Stefan Dösinger<stefandoesinger at gmx.at> wrote:
> Yay, Dplay work! A few suggestions though:
>
> From patch 1:
>> +static HRESULT WINAPI DPWSCB_GetCaps( LPDPSP_GETCAPSDATA data )
>> +{
>> +    TRACE( "(%d,%p,0x%08x,%p)\n",
>> +           data->idPlayer, data->lpCaps, data->dwFlags, data->lpISP );
>>+    return DP_OK;
>> +}
> Is there any reason this one writes a TRACE instread of a FIXME? I noticed
> that patch 3 implements it, perhaps you missed this when separating the
> patches?

Hum, true, I missed it, but anyway it gets implemented two patches later.

>> +static HRESULT WINAPI DPWSCB_Open( LPDPSP_OPENDATA data )
>> +{
>> +    FIXME( "(%u,%p,%p,%u,0x%08x,0x%08x) stub\n",
>> +           data->bCreate, data->lpSPMessageHeader, data->lpISP,
>> +           data->bReturnStatus, data->dwOpenFlags, data->dwSessionFlags );
>> +    return DP_OK;
>> +}
> Why does it return DP_OK while most other stubs return an error?

I implemented it a year ago and I don't remember clearly that detail,
but it most probably was just to avoid dpwsoxks initialization failing
so I could keep fixing invalid parameter detection inside dplayx.
Anyway same issue than above, the function is implemented but it will
come in the next series of patches, when I finish reviewing them.

> Patch 2:
>> +    /* Initialize internal data */
>> +    ZeroMemory( &dpwsData, sizeof(DPWS_DATA) );
> I think memset(&dpwsData, 0, sizeof(DPWS_DATA)) is preferred over ZeroMemory.

I wasn't informed, but the codebase seems to confirm it:

    [raziel at Bebop dlls]$ grep -Pr "memset\([^,]+,[ ]*0" * | wc -l
    5565
    [raziel at Bebop dlls]$ grep -r ZeroMemory * | wc -l
    660

That should probably go into the doc
(http://www.winehq.org/site/developer-cheatsheet), like the advice of
using HeapAlloc instead of malloc.

> Patch 3:
> Where do the values like dwMaxLocalPlayers = 65536 come from? Since most
> functions are still stubs its hard to see where it comes from? Does native
> dpwsockx have the same limits? (If it does, that's a solid reason for using
> the same limits)

They come from the tests results, I copied them from the original
implementation. Some of them have a documented behaviour (like
substracting the size of the headers from the max buffer size), but
others seem to be arbitrary. They may need some tweaking depending on
the Windows version, but that will be shown over time by further
testing.

Thanks for the feedback :)



More information about the wine-devel mailing list