Re: [RPCRT4] TCP server support (2/2) accept TCP connections

[Robert Shearman]

Damjan Jovanovic wrote:
> Adds support for RPC TCP servers using native (as opposed to winsock)
> sockets. Probably closes bug 2416.
>
> Is this any better, Robert?

The design looks good to me. I'll work on the provider infrastructure so 
that the extra thread isn't needed in the TCP case.

I think you'll need to rebase against current git as I had a patch 
committed recently which will have probably caused a conflict in your patch.

There are two further minor points with the patch, which I've 
highlighted below:
> Changelog:
> * added support for RPC TCP servers
>
> --- a/AUTHORS	2006-07-10 18:01:06.000000000 +0200
> +++ b/AUTHORS	2006-10-08 11:34:59.000000000 +0200
> @@ -873,3 +873,4 @@
>  Rizsanyi Zsolt
>  Per Ångström
>  Peter Ã…strand
> +Damjan Jovanovic

This list is sorted by surname, but more importantly is regenerated by 
Alexandre using a script every 6 months or so. I think you'd do better 
crediting yourself in the copyright lines in rpc_transport.c.
> +      thread = CreateThread(NULL, 0, rpcrt4_tcp_poll_thread, tcpc, 0, NULL);
> +      if (thread == NULL)
> +      {
> +        WARN("creating server polling thread failed, error %lu\n",
> +          GetLastError());
> +        tcpc->sock = -1;
> +      }
> +
> +    done:
> +      if (thread == NULL) /* ie. we failed somewhere */
> +      {
> +        close(sock);
> +        if (tcpc->onEventAvailable != NULL)
> +        {
> +          CloseHandle(tcpc->onEventAvailable);
> +          tcpc->onEventAvailable = NULL;
> +        }
> +        if (tcpc->onEventHandled != NULL)
> +        {
> +          CloseHandle(tcpc->onEventHandled);
> +          tcpc->onEventHandled = NULL;
> +        }
> +        continue;
> +      }
> +    }

You leak the thread handle.

-- 
Rob Shearman