dpnet patches

[Alistair Leslie-Hughes]

Hi Stefan,

On 31/01/2014 11:39 PM, Stefan Dösinger wrote:

> A while ago I promised to have a closer look at your dpnet patches. It’s a bit delayed, but here are some comments. How serious are you about dplay? Do you intend to do more work towards implementing it, or is that just wishful thinking on my part?

Thanks for you comments. I do plan on implementing as much as I can, but 
with limited time it's going to happen over a weekend.

>> -IMPORTS   = dpnet ole32
>> +IMPORTS   = ole32
> I’m not sure about this one, but winetest can automatically skip tests if the tested DLL doesn’t exist. It might use the imported libraries for that, or maybe just the name of the test. In the former case I think you want to keep the dpnet import.
>
> http://source.winehq.org/patches/data/101855:
> http://source.winehq.org/patches/data/101854:
> Did you think about sharing code between IDirectPlay8Peer and IDirectPlay8Server? The interfaces themselves have no relationship, but their methods are similar.
Yes, I did think that these two interfaces would need to share a lot of 
code.
>
> http://source.winehq.org/patches/data/101853:
>> +typedef struct IDirectPlay8ClientImpl
>> +{
>> +    IDirectPlay8Client IDirectPlay8Client_iface;
>> +    LONG ref;
>> +} IDirectPlay8ClientImpl;
> Are you sure you want to move the definition out of the library-wide header? If that works out ok it’s great, but you may need something other than the public interface in some other files in this library.
Sure,  I leave it where it for now.

>
> I recommend to work without the typedef, but that should be changed in a separate patch.
>
>> -    ULONG refCount = InterlockedIncrement(&This->ref);
>> +    ULONG ref = InterlockedIncrement(&This->ref);
>> -    TRACE("(%p)->(ref before=%u)\n", This, refCount - 1);
>> +    TRACE("(%p) ref=%d\n", This, ref);
> ULONG is unsigned, you have to use %u.
I'll fix that.

>
> I know that those always exist on Windows, but wouldn’t it be better to not advertise them until we have an actual implementation? I think advertising DPNSPWinsockTCP alone should be ok for the start.

Agreed, I'll adjust the register file.


Thanks for all you comments.

Alistair.