Re: wininet: Call WSAStartup/WSACleanup to mime the Windows behavior

[Thomas Faber]

Hey Bruno,
> +static void init_ws2_32(void)
> +{
> - static int once;
> - if (once++) return;
> - hws2_32 = GetModuleHandleA("ws2_32.dll");
> - pWSAStartup = (void *)GetProcAddress(hws2_32, "WSAStartup");
> - pWSACleanup = (void *)GetProcAddress(hws2_32, "WSACleanup");
>
> +}
> -
>
> +static void free_ws2_32(void)
> +{
> - FreeLibrary(hws2_32);
>
> +}

GetModuleHandle doesn't increase the reference count so you probably
shouldn't call FreeLibrary.
> - /* initialize winsock DLL here as expected by some broken applications */
> - if (!winsock_init++)
> - {
> -
>    char buffer[128];
> -
>    init_ws2_32();
> -
>    pWSAStartup(MAKEWORD( 1, 1 ), buffer);
> - }

I believe Windows initializes with version 2. At least that's what my
old test says (line 105):

https://code.reactos.org/browse/reactos/trunk/rostests/apitests/wininet/Inte...
(free to use under LGPL... I can make it explicit in the source file,
 or help with getting it in Winetest format if desired)

By the way, the same applies to winhttp in case you're interested:
https://code.reactos.org/browse/reactos/trunk/rostests/apitests/winhttp/WinH...

Thanks!
-Thomas

wininet: Call WSAStartup/WSACleanup to mime the Windows behavior

[Bruno Jesus]

On Fri, Jul 4, 2014 at 9:43 AM, Thomas Faber  wrote:
> Hey Bruno,
> > +static void init_ws2_32(void)
> > +{
> > - static int once;
> > - if (once++) return;
> > - hws2_32 = GetModuleHandleA("ws2_32.dll");
> > - pWSAStartup = (void *)GetProcAddress(hws2_32, "WSAStartup");
> > - pWSACleanup = (void *)GetProcAddress(hws2_32, "WSACleanup");
> >
> > +}
> > -
> >
> > +static void free_ws2_32(void)
> > +{
> > - FreeLibrary(hws2_32);
> >
> > +}
>
> GetModuleHandle doesn't increase the reference count so you probably
> shouldn't call FreeLibrary.

Thanks, I misread MSDN. It says that only GetModuleHandleEx require
the FreeLibrary call.
> > - /* initialize winsock DLL here as expected by some broken applications */
> > - if (!winsock_init++)
> > - {
> > -
> >    char buffer[128];
> > -
> >    init_ws2_32();
> > -
> >    pWSAStartup(MAKEWORD( 1, 1 ), buffer);
> > - }
>
> I believe Windows initializes with version 2. At least that's what my
> old test says (line 105):

I based the call in the relay log from native wininet:
0024:Call ws2_32.WSAStartup(00000101,0033e378) ret=76c16d16
> https://code.reactos.org/browse/reactos/trunk/rostests/apitests/wininet/Inte...
> (free to use under LGPL... I can make it explicit in the source file,
>  or help with getting it in Winetest format if desired)
>
> By the way, the same applies to winhttp in case you're interested:
> https://code.reactos.org/browse/reactos/trunk/rostests/apitests/winhttp/WinH...

Ok, thanks for the information.
> Thanks!
> -Thomas

Thanks again,
Bruno

wininet: Call WSAStartup/WSACleanup to mime the Windows behavior

[Alexandre Julliard]

Bruno Jesus  writes:
> > I believe Windows initializes with version 2. At least that's what my
> > old test says (line 105):
>
> I based the call in the relay log from native wininet:
> 0024:Call ws2_32.WSAStartup(00000101,0033e378) ret=76c16d16

This is not allowed, you can't look at the internal behavior of native
wininet. Now I can't accept your patch. You should have done this
through test cases only.

wininet: Call WSAStartup/WSACleanup to mime the Windows behavior

[Erich E. Hoover]

On Fri, Jul 4, 2014 at 7:10 AM, Alexandre Julliard 
wrote:
> Bruno Jesus  writes:
> > > I believe Windows initializes with version 2. At least that's what my
> > > old test says (line 105):
> >
> > I based the call in the relay log from native wininet:
> > 0024:Call ws2_32.WSAStartup(00000101,0033e378) ret=76c16d16
>
> This is not allowed, you can't look at the internal behavior of native
> wininet. Now I can't accept your patch. You should have done this
> through test cases only.

Technically he didn't.  As he said in the original email, Anastasius Focht
is the one who looked at the wininet behavior and described it others (
https://bugs.winehq.org/show_bug.cgi?id=36830#c0).  Sounds to me like the
old Compaq system of having one person look at what the original does while
having another person implement it from their notes.

Best,
Erich

wininet: Call WSAStartup/WSACleanup to mime the Windows behavior

[Vincent Povirk]

On Fri, Jul 4, 2014 at 10:56 AM, Erich E. Hoover
 wrote:
> As he said in the original email, Anastasius Focht is the one who looked at
> the wininet behavior and described it others
> (https://bugs.winehq.org/show_bug.cgi?id=36830#c0).  Sounds to me like the
> old Compaq system of having one person look at what the original does while
> having another person implement it from their notes.

The point of this system is to isolate the implementer from the
implementation details of the original, so they can't copy those
details. They are supposed to only be given an interface to implement,
which they may do differently (if that's possible while still
implementing the required interface). It doesn't work if the
implementer is told the implementation details, so when doing this you
have to be careful about the information you give.

I can't explain why the relay call out from wininet to winsock is too
much information in this case, but it's been a general policy that we
don't look at the calls native makes. (I also learned that policy the
hard way, unfortunately.)

wininet: Call WSAStartup/WSACleanup to mime the Windows behavior

[Erich E. Hoover]

On Sat, Jul 5, 2014 at 10:52 AM, Vincent Povirk  wrote:
> ...
> The point of this system is to isolate the implementer from the
> implementation details of the original, so they can't copy those
> details. They are supposed to only be given an interface to implement,
> which they may do differently (if that's possible while still
> implementing the required interface). It doesn't work if the
> implementer is told the implementation details, so when doing this you
> have to be careful about the information you give.

Yeah, I'm aware - I was actually surprised to find (after some
Googling as a result of this discussion) that the 9th circuit ruled in
favor of explicit copying in a case back in 2000 (
http://en.wikipedia.org/wiki/Sony_Computer_Entertainment,_Inc._v._Connectix_...
).  It was my understanding that US law (and case law) didn't have any
precedent for permitting something like that.  Anyway, it's probably
still a good idea to stick to strict clean room techniques.
> I can't explain why the relay call out from wininet to winsock is too
> much information in this case, but it's been a general policy that we
> don't look at the calls native makes. (I also learned that policy the
> hard way, unfortunately.)

Maybe we should update http://wiki.winehq.org/CleanRoomGuidelines and
make it more prominent on http://www.winehq.org/devel/ ?  I don't even
know how to get to it from that page, and it seems like a pretty
important document...

Best,
Erich

wininet: Call WSAStartup/WSACleanup to mime the Windows behavior

[Joris van der Wel]

First section on http://wiki.winehq.org/SubmittingPatches ;)
http://wiki.winehq.org/SubmittingPatches
> Maybe we should update http://wiki.winehq.org/CleanRoomGuidelines and
> make it more prominent on http://www.winehq.org/devel/ ?  I don't even
> know how to get to it from that page, and it seems like a pretty
> important document...
>
> Best,
> Erich

wininet: Call WSAStartup/WSACleanup to mime the Windows behavior

[Erich E. Hoover]

On Sun, Jul 6, 2014 at 12:32 PM, Joris van der Wel
 wrote:
> First section on http://wiki.winehq.org/SubmittingPatches ;)

Ah, so it is - clearly everyone's not seeing it though ;)

Best,
Erich