development efforts on the Solaris side.

[Poul-Henning Kamp]

In message <E01C67F9-72C5-49C4-81DC-61410014E6E1 at omniti.com>, Theo Schlossnagle
 writes:

>> Can you mail me a link to the patch ?
>
>http://lethargy.org/~jesus/misc/varnish-solaris-trunk-2328.diff

Ok, various comments:

The autoconf stuff and the #inclusion of "varnish_config.h" all over
the place you will have to negotiate with DES, I only do C code.

lib/libvarnish/time.c:

	I don't like #ifdefs like that in the middle of functions.
	The appropriate way is to add a compat function in
	libvarnishcompat.

flock/fcntl-locks you're already talking with DES about.

curses:
	What exactly is the semantics of KEY_RESIZE ?

mgt_vcc.c:
	The suffix shouldn't matter to dlopen(), please explain ?
	White-space and {} spam.
	Adding the c-compiler command to the error message is bound
	to make it overflow the linewidth.

VCL_WaitForActive()
	Need to look at that in more detail.  I deliberately tried
	to limit varnish to only use mutexes for locking, so introducing
	semaphores just for this seems somewhat silly.

storage_umem.c
	I'm not sure I see much point in this.  The main advantage of
	umem is SMP localized storage management, and the Varnish
	objects are exactly not local to any one CPU.
	Benchmarks could possibly convince me otherwise.

#include <err.h>
	This may be a portability issue where the easiest way to fix
	is to avoid it.

sendfile():
	Does the solaris sendfile guarantee that storage is no longer
	touched when it returns ?  Otherwise it's as little use as
	the FreeBSD and Linux versions.

/* pick a stevedore and bump the head along */
/* XXX: only safe as long as pointer writes are atomic */
+/* jesus: dear god, are you crazy? */
stv = stevedores = stevedores->next;

	God to Jesus:  No I'm not.

-/* Note: intentionally not IOV_MAX */
+#ifdef IOV_MAX
+#define MAX_IOVS	IOV_MAX
+#else
 #define MAX_IOVS	(HTTP_HDR_MAX * 2)
+#endif

	Which bit of "intentionally" didn't you understand ?
	Have you examined the value of IOV_MAX, looked at where
	this is used and measured the impact ?

cache_acceptor.c
	Under no circumstances should #ifdef HAVE_PORT_CREATE be
	necessary here.  If a new method is necessary for the
	acceptors, so be it.

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk at FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe    
Never attribute to malice what can adequately be explained by incompetence.