Re: [PATCH] Multi-threaded svnserve

Neat.

> (with the exception of basic_tests 11, but that fails even without
> this patch)

That's odd. The tests all worked for me last time I ran the test
suite. (Though that was before CMike's latest round of changes.)

Some style nits:

> +#if APR_HAS_FORK
> +/* ### for testing threading is enabled even when fork is available */
> +#define SVNSERVE_FORK 0
> +#define SVNSERVE_THREAD 1
> +#elif APR_HAS_THREADS
> +#define SVNSERVE_FORK 0
> +#define SVNSERVE_THREAD 1
> +#endif

We appear to mostly use #ifdef in Subversion, not #if, and I prefer that
style anyway.

> +static void * APR_THREAD_FUNC
> +serve_thread(apr_thread_t *tid,
> + void *data)

In my files, there are no newlines in function definitions unless
they're necessary to keep lines at 79 columns or less.

I'm not entirely happy with the amount of preprocessor chicken scratch
which winds up in the main loop with this change. It would be nice to
refactor things so that there is less. But that's only a -0.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Jun 2 06:45:35 2003