[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Next rev of Windows service support (Feb 27)

From: Branko Čibej <brane_at_xbc.nu>
Date: 2006-02-28 05:57:36 CET

Arlie Davis wrote:
> I have intentionally not altered INSTALL, because it is not clear whether I
> should add a link to windows-service.txt, or simply move the
> windows-service.txt into INSTALL. I think the best thing is to check in
> windows-service.txt as is, and then massage the docs later, as people see
> fit. This patch is intended to get the basic service support checked in and
> available to people, while future patches will work on getting all of the
> docs, installation apps, etc. plugged in.
>
Makes sense.
[snip]

> +You can create as many services as you need; there is no restriction on
> +the number of services, or their names. I use a prefix, like "svn.foo",
> +"svn.bar", etc. Each service runs in a separate process. As usual,
> +it is your responsbility as an administrator to make sure that no
> +two service instances use the same repository root path, or the same
> +combination of --listen-port and --listen-host.
>
Actually, the repository root path _can_ be the same. You can have any
number of processes accessing the same repository.
[snip]

> +#ifdef _WIN32
> +#include <arch/win32/apr_arch_networkio.h>
>
I don't believe we actually need this include any longer, do we?

> +static HANDLE winservice_accept_socket_handle;
>
Should be "apr_os_socket_t", not "HANDLE" -- that way, we can reuse it
on other platforms if/when appropriate.
[snip]

> +void winservice_notify_stop()
> +{
> + CloseHandle((HANDLE)winservice_accept_socket_handle);
> +}
>
The cast isn't necessary, winservice_accept_socket_handle is already the
correct type.

Question: A bit lower down, you set this variable to NULL if
apr_os_sock_get fails, but you don't check for NULL here. Is that safe?

This looks good. If nobody objects, I'll commit this after some testing.

Just FYI, the indentation in winservice.c is wrong ... we indent the
braces, too, not just the block contents. I can do that automagically
before I commit the patch.

I still think we can do away with most of the new #ifdefs in main.c, and
I'll probably get rid of them in a subsequent commit.

-- Brane

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Feb 28 05:57:57 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.