Re: [PATCH try4] wininet: Added support for persistent HTTP connections

[Alexandre Julliard]

Jacek Caban  writes:
> -
>    /* FIXME: do the check also for SSL connections */
> -
>    if(!netconn->useSSL) {
> -
>        res = NETCON_recv(netconn, &b, 1, MSG_PEEK|MSG_DONTWAIT, &len);
> -
>        if((res == ERROR_SUCCESS && len == 1) || res == WSAEWOULDBLOCK)
> -
>            break;

This doesn't build with winsock (MSG_DONTWAIT is not defined). I'm also
not sure what is the purpose of that check. If it's to check if remote
has closed the connection, you need to do that by handling the error on
the first send.

[PATCH try4] wininet: Added support for persistent HTTP connections

[Jacek Caban]

On 05/06/11 13:17, Alexandre Julliard wrote:
> Jacek Caban  writes:
> > -
> >    /* FIXME: do the check also for SSL connections */
> > -
> >    if(!netconn->useSSL) {
> > -
> >        res = NETCON_recv(netconn, &b, 1, MSG_PEEK|MSG_DONTWAIT, &len);
> > -
> >        if((res == ERROR_SUCCESS && len == 1) || res == WSAEWOULDBLOCK)
> > -
> >            break;
>
> This doesn't build with winsock (MSG_DONTWAIT is not defined). I'm also
> not sure what is the purpose of that check. If it's to check if remote
> has closed the connection, you need to do that by handling the error on
> the first send.

Yes, the check is supposed to test for closed connections. It's inspired
in how it's done in Gecko [1]. Handling error on the first send also
should be fixed (it needs more investigation, send successes for me,
it's later recv that fails and it's already too late to handle it well),
but we should attempt to detect closed connection earlier because
sending has side effects like callback notifications.

Thanks,
    Jacek

[1]
http://wine.git.sourceforge.net/git/gitweb.cgi?p=wine/wine-gecko;a=blob;f=ne...