[PATCH/RFC 00/11] daemon-win32

[PATCH/RFC 00/11] daemon-win32

[Erik Faye-Lund]

This is my stab at cleaning up Mike Pape's patches for git-daemon
on Windows for submission, plus some of my own.

* Patch 1-4 originate from Mike Pape, but have been cleaned up quite
  a lot by me. I hope Mike won't hate me. Credit have been retained,
  as all important code here were written by Mike. The commit
  messages were written mostly by me.

* Patch 5 is a trivial old-style function declaration fix.

* Patch 6 introduces two new functions to the run-command API,
  kill_async() and is_async_alive().

* Patch 7-8 removes the stdin/stdout redirection for the service
  functions, as redirecting won't work for the threaded version.

* Patch 9 converts the daemon-code to use the run-command API. This
  is the patch I expect to be the most controversial.

* Patch 10 is about using line-buffered mode instead of full-buffered
  mode for stderr.

* Patch 11 finally enables compilation of git-daemon on MinGW.

Let the flames begin!

Erik Faye-Lund (7):
  inet_ntop: fix a couple of old-style decls
  run-command: add kill_async() and is_async_alive()
  run-command: support input-fd
  daemon: use explicit file descriptor
  daemon: use run-command api for async serving
  daemon: use full buffered mode for stderr
  mingw: compile git-daemon

Mike Pape (4):
  mingw: add network-wrappers for daemon
  strbuf: add non-variadic function strbuf_vaddf()
  mingw: implement syslog
  compat: add inet_pton and inet_ntop prototypes

 Makefile           |    8 ++-
 compat/inet_ntop.c |   22 ++------
 compat/inet_pton.c |    8 ++-
 compat/mingw.c     |  111 +++++++++++++++++++++++++++++++++++++++++--
 compat/mingw.h     |   32 ++++++++++++
 daemon.c           |  134 ++++++++++++++++++++++++++--------------------------
 git-compat-util.h  |    9 ++++
 run-command.c      |   32 ++++++++++++-
 run-command.h      |    2 +
 strbuf.c           |   15 ++++--
 strbuf.h           |    1 +
 11 files changed, 274 insertions(+), 100 deletions(-)

[PATCH/RFC 01/11] mingw: add network-wrappers for daemon

[Erik Faye-Lund]

From: Mike Pape <dotzenlabs@gmail.com>

git-daemon requires some socket-functionality that is not yet
supported in the Windows-port. This patch adds said functionality,
and makes sure WSAStartup gets called by socket(), since it is the
first network-call in git-daemon. In addition, a check is added to
prevent WSAStartup (and WSACleanup, though atexit) from being
called more than once, since git-daemon calls both socket() and
gethostbyname().

Signed-off-by: Mike Pape <dotzenlabs@gmail.com>
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/mingw.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 compat/mingw.h |   16 ++++++++++++++
 2 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 10d6796..458021b 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -983,23 +983,37 @@ char **make_augmented_environ(const char *const *vars)
 	return env;
 }
 
-/* this is the first function to call into WS_32; initialize it */
-#undef gethostbyname
-struct hostent *mingw_gethostbyname(const char *host)
+static void wsa_init(void)
 {
+	static int initialized = 0;
 	WSADATA wsa;
 
+	if (initialized)
+		return;
+
 	if (WSAStartup(MAKEWORD(2,2), &wsa))
 		die("unable to initialize winsock subsystem, error %d",
 			WSAGetLastError());
 	atexit((void(*)(void)) WSACleanup);
+	initialized = 1;
+}
+
+/* this can be the first function to call into WS_32; initialize it */
+#undef gethostbyname
+struct hostent *mingw_gethostbyname(const char *host)
+{
+	wsa_init();
 	return gethostbyname(host);
 }
 
+/* this can be the first function to call into WS_32; initialize it */
 int mingw_socket(int domain, int type, int protocol)
 {
+	SOCKET s;
 	int sockfd;
-	SOCKET s = WSASocket(domain, type, protocol, NULL, 0, 0);
+
+	wsa_init();
+	s = WSASocket(domain, type, protocol, NULL, 0, 0);
 	if (s == INVALID_SOCKET) {
 		/*
 		 * WSAGetLastError() values are regular BSD error codes
@@ -1029,6 +1043,44 @@ int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz)
 	return connect(s, sa, sz);
 }
 
+#undef bind
+int mingw_bind(int sockfd, struct sockaddr *sa, size_t sz)
+{
+	SOCKET s = (SOCKET)_get_osfhandle(sockfd);
+	return bind(s, sa, sz);
+}
+
+#undef setsockopt
+int mingw_setsockopt(int sockfd, int lvl, int optname, void *optval, int optlen)
+{
+	SOCKET s = (SOCKET)_get_osfhandle(sockfd);
+	return setsockopt(s, lvl, optname, (const char*)optval, optlen);
+}
+
+#undef listen
+int mingw_listen(int sockfd, int backlog)
+{
+	SOCKET s = (SOCKET)_get_osfhandle(sockfd);
+	return listen(s, backlog);
+}
+
+#undef accept
+int mingw_accept(int sockfd1, struct sockaddr *sa, socklen_t *sz)
+{
+	int sockfd2;
+
+	SOCKET s1 = (SOCKET)_get_osfhandle(sockfd1);
+	SOCKET s2 = accept(s1, sa, sz);
+
+	/* convert into a file descriptor */
+	if ((sockfd2 = _open_osfhandle(s2, O_RDWR|O_BINARY)) < 0) {
+		closesocket(s2);
+		return error("unable to make a socket file descriptor: %s",
+			strerror(errno));
+	}
+	return sockfd2;
+}
+
 #undef rename
 int mingw_rename(const char *pold, const char *pnew)
 {
diff --git a/compat/mingw.h b/compat/mingw.h
index 1978f8a..f362f61 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -5,6 +5,7 @@
  */
 
 typedef int pid_t;
+typedef int socklen_t;
 #define hstrerror strerror
 
 #define S_IFLNK    0120000 /* Symbolic link */
@@ -33,6 +34,9 @@ typedef int pid_t;
 #define F_SETFD 2
 #define FD_CLOEXEC 0x1
 
+#define EAFNOSUPPORT WSAEAFNOSUPPORT
+#define ECONNABORTED WSAECONNABORTED
+
 struct passwd {
 	char *pw_name;
 	char *pw_gecos;
@@ -182,6 +186,18 @@ int mingw_socket(int domain, int type, int protocol);
 int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz);
 #define connect mingw_connect
 
+int mingw_bind(int sockfd, struct sockaddr *sa, size_t sz);
+#define bind mingw_bind
+
+int mingw_setsockopt(int sockfd, int lvl, int optname, void *optval, int optlen);
+#define setsockopt mingw_setsockopt
+
+int mingw_listen(int sockfd, int backlog);
+#define listen mingw_listen
+
+int mingw_accept(int sockfd, struct sockaddr *sa, socklen_t *sz);
+#define accept mingw_accept
+
 int mingw_rename(const char*, const char*);
 #define rename mingw_rename
 
-- 
1.6.5.rc2.7.g4f8d3

[PATCH/RFC 02/11] strbuf: add non-variadic function strbuf_vaddf()

[Erik Faye-Lund]

From: Mike Pape <dotzenlabs@gmail.com>

This patch adds strbuf_vaddf, which takes a va_list as input
instead of the variadic input that strbuf_addf takes. This
is useful for fowarding varargs to strbuf_addf.

Signed-off-by: Mike Pape <dotzenlabs@gmail.com>
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 strbuf.c |   15 +++++++++------
 strbuf.h |    1 +
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/strbuf.c b/strbuf.c
index a6153dc..e1833fb 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -190,23 +190,18 @@ void strbuf_adddup(struct strbuf *sb, size_t pos, size_t len)
 	strbuf_setlen(sb, sb->len + len);
 }
 
-void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
+void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap)
 {
 	int len;
-	va_list ap;
 
 	if (!strbuf_avail(sb))
 		strbuf_grow(sb, 64);
-	va_start(ap, fmt);
 	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
-	va_end(ap);
 	if (len < 0)
 		die("your vsnprintf is broken");
 	if (len > strbuf_avail(sb)) {
 		strbuf_grow(sb, len);
-		va_start(ap, fmt);
 		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
-		va_end(ap);
 		if (len > strbuf_avail(sb)) {
 			die("this should not happen, your snprintf is broken");
 		}
@@ -214,6 +209,14 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
 	strbuf_setlen(sb, sb->len + len);
 }
 
+void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
+{
+	va_list va;
+	va_start(va, fmt);
+	strbuf_vaddf(sb, fmt, va);
+	va_end(va);
+}
+
 void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn,
 		   void *context)
 {
diff --git a/strbuf.h b/strbuf.h
index d05e056..8686bcb 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -119,6 +119,7 @@ extern size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
 
 __attribute__((format(printf,2,3)))
 extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);
+extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
 
 extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
 /* XXX: if read fails, any partial read is undone */
-- 
1.6.5.rc2.7.g4f8d3

[PATCH/RFC 03/11] mingw: implement syslog

[Erik Faye-Lund]

From: Mike Pape <dotzenlabs@gmail.com>

Syslog does not usually exist on Windows, so we implement our own
using Window's ReportEvent mechanism.

Signed-off-by: Mike Pape <dotzenlabs@gmail.com>
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/mingw.c    |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 compat/mingw.h    |   15 +++++++++++++++
 daemon.c          |    2 --
 git-compat-util.h |    1 +
 4 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 458021b..68116ac 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1255,6 +1255,57 @@ int sigaction(int sig, struct sigaction *in, struct sigaction *out)
 	return 0;
 }
 
+static HANDLE ms_eventlog;
+
+void openlog(const char *ident, int logopt, int facility) {
+	if (ms_eventlog)
+		return;
+	ms_eventlog = RegisterEventSourceA(NULL, ident);
+}
+
+void syslog(int priority, const char *fmt, ...) {
+	struct strbuf msg;
+	va_list va;
+	WORD logtype;
+
+	strbuf_init(&msg, 0);
+	va_start(va, fmt);
+	strbuf_vaddf(&msg, fmt, va);
+	va_end(va);
+
+	switch (priority) {
+		case LOG_EMERG:
+		case LOG_ALERT:
+		case LOG_CRIT:
+		case LOG_ERR:
+			logtype = EVENTLOG_ERROR_TYPE;
+			break;
+
+		case LOG_WARNING:
+			logtype = EVENTLOG_WARNING_TYPE;
+			break;
+
+		case LOG_NOTICE:
+		case LOG_INFO:
+		case LOG_DEBUG:
+		default:
+			logtype = EVENTLOG_INFORMATION_TYPE;
+			break;
+	}
+
+	ReportEventA(ms_eventlog,
+	    logtype,
+	    (WORD)NULL,
+	    (DWORD)NULL,
+	    NULL,
+	    1,
+	    0,
+	    (const char **)&msg.buf,
+	    NULL);
+
+	strbuf_release(&msg);
+}
+
 #undef signal
 sig_handler_t mingw_signal(int sig, sig_handler_t handler)
 {
diff --git a/compat/mingw.h b/compat/mingw.h
index f362f61..576b1a1 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -37,6 +37,19 @@ typedef int socklen_t;
 #define EAFNOSUPPORT WSAEAFNOSUPPORT
 #define ECONNABORTED WSAECONNABORTED
 
+#define LOG_PID     0x01
+
+#define LOG_EMERG   0
+#define LOG_ALERT   1
+#define LOG_CRIT    2
+#define LOG_ERR     3
+#define LOG_WARNING 4
+#define LOG_NOTICE  5
+#define LOG_INFO    6
+#define LOG_DEBUG   7
+
+#define LOG_DAEMON  (3<<3)
+
 struct passwd {
 	char *pw_name;
 	char *pw_gecos;
@@ -157,6 +170,8 @@ int sigaction(int sig, struct sigaction *in, struct sigaction *out);
 int link(const char *oldpath, const char *newpath);
 int symlink(const char *oldpath, const char *newpath);
 int readlink(const char *path, char *buf, size_t bufsiz);
+void openlog(const char *ident, int logopt, int facility);
+void syslog(int priority, const char *fmt, ...);
 
 /*
  * replacements of existing functions
diff --git a/daemon.c b/daemon.c
index 1b5ada6..07d7356 100644
--- a/daemon.c
+++ b/daemon.c
@@ -4,8 +4,6 @@
 #include "run-command.h"
 #include "strbuf.h"
 
-#include <syslog.h>
-
 #ifndef HOST_NAME_MAX
 #define HOST_NAME_MAX 256
 #endif
diff --git a/git-compat-util.h b/git-compat-util.h
index ef60803..33a8e33 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -105,6 +105,7 @@
 #include <netdb.h>
 #include <pwd.h>
 #include <inttypes.h>
+#include <syslog.h>
 #if defined(__CYGWIN__)
 #undef _XOPEN_SOURCE
 #include <grp.h>
-- 
1.6.5.rc2.7.g4f8d3

[PATCH/RFC 04/11] compat: add inet_pton and inet_ntop prototypes

[Erik Faye-Lund]

From: Mike Pape <dotzenlabs@gmail.com>

Windows doesn't have inet_pton and inet_ntop, so
add prototypes in git-compat-util.h for them.

At the same time include git-compat-util.h in
the sources for these functions, so they use the
network-wrappers from there on Windows.

Signed-off-by: Mike Pape <dotzenlabs@gmail.com>
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 Makefile           |    2 ++
 compat/inet_ntop.c |    6 +++---
 compat/inet_pton.c |    8 +++++---
 git-compat-util.h  |    8 ++++++++
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 70cee6d..3b01694 100644
--- a/Makefile
+++ b/Makefile
@@ -1227,9 +1227,11 @@ endif
 endif
 ifdef NO_INET_NTOP
 	LIB_OBJS += compat/inet_ntop.o
+	BASIC_CFLAGS += -DNO_INET_NTOP
 endif
 ifdef NO_INET_PTON
 	LIB_OBJS += compat/inet_pton.o
+	BASIC_CFLAGS += -DNO_INET_PTON
 endif
 
 ifdef NO_ICONV
diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c
index f444982..e5b46a0 100644
--- a/compat/inet_ntop.c
+++ b/compat/inet_ntop.c
@@ -17,9 +17,9 @@
 
 #include <errno.h>
 #include <sys/types.h>
-#include <sys/socket.h>
-#include <netinet/in.h>
-#include <arpa/inet.h>
+
+#include "../git-compat-util.h"
+
 #include <stdio.h>
 #include <string.h>
 
diff --git a/compat/inet_pton.c b/compat/inet_pton.c
index 4078fc0..2ec995e 100644
--- a/compat/inet_pton.c
+++ b/compat/inet_pton.c
@@ -17,9 +17,9 @@
 
 #include <errno.h>
 #include <sys/types.h>
-#include <sys/socket.h>
-#include <netinet/in.h>
-#include <arpa/inet.h>
+
+#include "../git-compat-util.h"
+
 #include <stdio.h>
 #include <string.h>
 
@@ -41,7 +41,9 @@
  */
 
 static int inet_pton4(const char *src, unsigned char *dst);
+#ifndef NO_IPV6
 static int inet_pton6(const char *src, unsigned char *dst);
+#endif
 
 /* int
  * inet_pton4(src, dst)
diff --git a/git-compat-util.h b/git-compat-util.h
index 33a8e33..27fa601 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -340,6 +340,14 @@ static inline char *gitstrchrnul(const char *s, int c)
 }
 #endif
 
+#ifdef NO_INET_PTON
+int inet_pton(int af, const char *src, void *dst);
+#endif
+
+#ifdef NO_INET_NTOP
+const char *inet_ntop(int af, const void *src, char *dst, size_t size);
+#endif
+
 extern void release_pack_memory(size_t, int);
 
 extern char *xstrdup(const char *str);
-- 
1.6.5.rc2.7.g4f8d3

[PATCH/RFC 05/11] inet_ntop: fix a couple of old-style decls

[Erik Faye-Lund]

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/inet_ntop.c |   16 +++-------------
 1 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c
index e5b46a0..ea249c6 100644
--- a/compat/inet_ntop.c
+++ b/compat/inet_ntop.c
@@ -50,10 +50,7 @@
  *	Paul Vixie, 1996.
  */
 static const char *
-inet_ntop4(src, dst, size)
-	const u_char *src;
-	char *dst;
-	size_t size;
+inet_ntop4(const u_char *src, char *dst, size_t size)
 {
 	static const char fmt[] = "%u.%u.%u.%u";
 	char tmp[sizeof "255.255.255.255"];
@@ -78,10 +75,7 @@ inet_ntop4(src, dst, size)
  *	Paul Vixie, 1996.
  */
 static const char *
-inet_ntop6(src, dst, size)
-	const u_char *src;
-	char *dst;
-	size_t size;
+inet_ntop6(const u_char *src, char *dst, size_t size)
 {
 	/*
 	 * Note that int32_t and int16_t need only be "at least" large enough
@@ -178,11 +172,7 @@ inet_ntop6(src, dst, size)
  *	Paul Vixie, 1996.
  */
 const char *
-inet_ntop(af, src, dst, size)
-	int af;
-	const void *src;
-	char *dst;
-	size_t size;
+inet_ntop(int af, const void *src, char *dst, size_t size)
 {
 	switch (af) {
 	case AF_INET:
-- 
1.6.5.rc2.7.g4f8d3

[PATCH/RFC 06/11] run-command: add kill_async() and is_async_alive()

[Erik Faye-Lund]

These functions will aid the Windows port of git-daemon.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 run-command.c |   27 +++++++++++++++++++++++++++
 run-command.h |    2 ++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/run-command.c b/run-command.c
index cf2d8f7..e5a0e06 100644
--- a/run-command.c
+++ b/run-command.c
@@ -373,6 +373,33 @@ int finish_async(struct async *async)
 	return ret;
 }
 
+int kill_async(struct async *async)
+{
+#ifndef WIN32
+	return kill(async->pid, SIGTERM);
+#else
+	DWORD ret = 0;
+	if (!TerminateThread(async->tid, 0))
+		ret = error("killing thread failed: %lu", GetLastError());
+	else if (!GetExitCodeThread(async->tid, &ret))
+		ret = error("cannot get thread exit code: %lu", GetLastError());
+	return ret;
+#endif
+}
+
+int is_async_alive(struct async *async)
+{
+#ifndef WIN32
+	int dummy;
+	return waitpid(async->pid, &dummy, WNOHANG);
+#else
+	int ret = WaitForSingleObject(async->tid, 0);
+	if (ret == WAIT_FAILED)
+		return error("checking thread state failed: %lu", GetLastError());
+	return ret != WAIT_OBJECT_0;
+#endif
+}
+
 int run_hook(const char *index_file, const char *name, ...)
 {
 	struct child_process hook;
diff --git a/run-command.h b/run-command.h
index fb34209..955b0bd 100644
--- a/run-command.h
+++ b/run-command.h
@@ -80,5 +80,7 @@ struct async {
 
 int start_async(struct async *async);
 int finish_async(struct async *async);
+int kill_async(struct async *async);
+int is_async_alive(struct async *async);
 
 #endif
-- 
1.6.5.rc2.7.g4f8d3

[PATCH/RFC 07/11] run-command: support input-fd

[Erik Faye-Lund]

This patch adds the possibility to supply a non-0
file descriptor for communucation, instead of the
default-created pipe. The pipe gets duplicated, so
the caller can free it's handles.

This is usefull for async communication over sockets.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 run-command.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/run-command.c b/run-command.c
index e5a0e06..98771ef 100644
--- a/run-command.c
+++ b/run-command.c
@@ -327,7 +327,10 @@ int start_async(struct async *async)
 {
 	int pipe_out[2];
 
-	if (pipe(pipe_out) < 0)
+	if (async->out) {
+		pipe_out[0] = dup(async->out);
+		pipe_out[1] = dup(async->out);
+	} else if (pipe(pipe_out) < 0)
 		return error("cannot create pipe: %s", strerror(errno));
 	async->out = pipe_out[0];
 
-- 
1.6.5.rc2.7.g4f8d3

[PATCH/RFC 08/11] daemon: use explicit file descriptor

[Erik Faye-Lund]

This patch adds support to specify an explicit file
descriotor for communication with the client, instead
of using stdin/stdout.

This will be useful for the Windows port, because it
will use threads instead of fork() to serve multiple
clients, making it impossible to reuse stdin/stdout.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 daemon.c |   34 ++++++++++++++++------------------
 1 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/daemon.c b/daemon.c
index 07d7356..a0aead5 100644
--- a/daemon.c
+++ b/daemon.c
@@ -263,7 +263,7 @@ static char *path_ok(char *directory)
 	return NULL;		/* Fallthrough. Deny by default */
 }
 
-typedef int (*daemon_service_fn)(void);
+typedef int (*daemon_service_fn)(int);
 struct daemon_service {
 	const char *name;
 	const char *config_name;
@@ -287,7 +287,7 @@ static int git_daemon_config(const char *var, const char *value, void *cb)
 	return 0;
 }
 
-static int run_service(char *dir, struct daemon_service *service)
+static int run_service(int fd, char *dir, struct daemon_service *service)
 {
 	const char *path;
 	int enabled = service->enabled;
@@ -340,7 +340,7 @@ static int run_service(char *dir, struct daemon_service *service)
 	 */
 	signal(SIGTERM, SIG_IGN);
 
-	return service->fn();
+	return service->fn(fd);
 }
 
 static void copy_to_log(int fd)
@@ -364,7 +364,7 @@ static void copy_to_log(int fd)
 	fclose(fp);
 }
 
-static int run_service_command(const char **argv)
+static int run_service_command(int fd, const char **argv)
 {
 	struct child_process cld;
 
@@ -372,37 +372,35 @@ static int run_service_command(const char **argv)
 	cld.argv = argv;
 	cld.git_cmd = 1;
 	cld.err = -1;
+	cld.in = cld.out = fd;
 	if (start_command(&cld))
 		return -1;
 
-	close(0);
-	close(1);
-
 	copy_to_log(cld.err);
 
 	return finish_command(&cld);
 }
 
-static int upload_pack(void)
+static int upload_pack(int fd)
 {
 	/* Timeout as string */
 	char timeout_buf[64];
 	const char *argv[] = { "upload-pack", "--strict", timeout_buf, ".", NULL };
 
 	snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout);
-	return run_service_command(argv);
+	return run_service_command(fd, argv);
 }
 
-static int upload_archive(void)
+static int upload_archive(int fd)
 {
 	static const char *argv[] = { "upload-archive", ".", NULL };
-	return run_service_command(argv);
+	return run_service_command(fd, argv);
 }
 
-static int receive_pack(void)
+static int receive_pack(int fd)
 {
 	static const char *argv[] = { "receive-pack", ".", NULL };
-	return run_service_command(argv);
+	return run_service_command(fd, argv);
 }
 
 static struct daemon_service daemon_service[] = {
@@ -532,7 +530,7 @@ static void parse_host_arg(char *extra_args, int buflen)
 }
 
 
-static int execute(struct sockaddr *addr)
+static int execute(int fd, struct sockaddr *addr)
 {
 	static char line[1000];
 	int pktlen, len, i;
@@ -565,7 +563,7 @@ static int execute(struct sockaddr *addr)
 	}
 
 	alarm(init_timeout ? init_timeout : timeout);
-	pktlen = packet_read_line(0, line, sizeof(line));
+	pktlen = packet_read_line(fd, line, sizeof(line));
 	alarm(0);
 
 	len = strlen(line);
@@ -597,7 +595,7 @@ static int execute(struct sockaddr *addr)
 			 * Note: The directory here is probably context sensitive,
 			 * and might depend on the actual service being performed.
 			 */
-			return run_service(line + namelen + 5, s);
+			return run_service(fd, line + namelen + 5, s);
 		}
 	}
 
@@ -713,7 +711,7 @@ static void handle(int incoming, struct sockaddr *addr, int addrlen)
 	dup2(incoming, 1);
 	close(incoming);
 
-	exit(execute(addr));
+	exit(execute(0, addr));
 }
 
 static void child_handler(int signo)
@@ -1150,7 +1148,7 @@ int main(int argc, char **argv)
 		if (getpeername(0, peer, &slen))
 			peer = NULL;
 
-		return execute(peer);
+		return execute(0, peer);
 	}
 
 	if (detach) {
-- 
1.6.5.rc2.7.g4f8d3

[PATCH/RFC 09/11] daemon: use run-command api for async serving

[Erik Faye-Lund]

fork() is only available on POSIX, so to support git-daemon
on Windows we have to use something else. Conveniently
enough, we have an API for async operation already.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 daemon.c |   79 ++++++++++++++++++++++++++++---------------------------------
 1 files changed, 36 insertions(+), 43 deletions(-)

diff --git a/daemon.c b/daemon.c
index a0aead5..1b0e290 100644
--- a/daemon.c
+++ b/daemon.c
@@ -372,7 +372,8 @@ static int run_service_command(int fd, const char **argv)
 	cld.argv = argv;
 	cld.git_cmd = 1;
 	cld.err = -1;
-	cld.in = cld.out = fd;
+	cld.in = dup(fd);
+	cld.out = fd;
 	if (start_command(&cld))
 		return -1;
 
@@ -609,11 +610,11 @@ static unsigned int live_children;
 
 static struct child {
 	struct child *next;
-	pid_t pid;
+	struct async async;
 	struct sockaddr_storage address;
 } *firstborn;
 
-static void add_child(pid_t pid, struct sockaddr *addr, int addrlen)
+static void add_child(struct async *async, struct sockaddr *addr, int addrlen)
 {
 	struct child *newborn, **cradle;
 
@@ -623,7 +624,7 @@ static void add_child(pid_t pid, struct sockaddr *addr, int addrlen)
 	 */
 	newborn = xcalloc(1, sizeof(*newborn));
 	live_children++;
-	newborn->pid = pid;
+	memcpy(&newborn->async, async, sizeof(struct async));
 	memcpy(&newborn->address, addr, addrlen);
 	for (cradle = &firstborn; *cradle; cradle = &(*cradle)->next)
 		if (!memcmp(&(*cradle)->address, &newborn->address,
@@ -633,19 +634,6 @@ static void add_child(pid_t pid, struct sockaddr *addr, int addrlen)
 	*cradle = newborn;
 }
 
-static void remove_child(pid_t pid)
-{
-	struct child **cradle, *blanket;
-
-	for (cradle = &firstborn; (blanket = *cradle); cradle = &blanket->next)
-		if (blanket->pid == pid) {
-			*cradle = blanket->next;
-			live_children--;
-			free(blanket);
-			break;
-		}
-}
-
 /*
  * This gets called if the number of connections grows
  * past "max_connections".
@@ -654,7 +642,7 @@ static void remove_child(pid_t pid)
  */
 static void kill_some_child(void)
 {
-	const struct child *blanket, *next;
+	struct child *blanket, *next;
 
 	if (!(blanket = firstborn))
 		return;
@@ -662,28 +650,37 @@ static void kill_some_child(void)
 	for (; (next = blanket->next); blanket = next)
 		if (!memcmp(&blanket->address, &next->address,
 			    sizeof(next->address))) {
-			kill(blanket->pid, SIGTERM);
+			kill_async(&blanket->async);
 			break;
 		}
 }
 
 static void check_dead_children(void)
 {
-	int status;
-	pid_t pid;
-
-	while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
-		const char *dead = "";
-		remove_child(pid);
-		if (!WIFEXITED(status) || (WEXITSTATUS(status) > 0))
-			dead = " (with error)";
-		loginfo("[%"PRIuMAX"] Disconnected%s", (uintmax_t)pid, dead);
-	}
+	struct child **cradle, *blanket;
+	for (cradle = &firstborn; (blanket = *cradle);)
+		if (!is_async_alive(&blanket->async)) {
+			*cradle = blanket->next;
+			loginfo("Disconnected\n");
+			live_children--;
+			close(blanket->async.out);
+			free(blanket);
+		} else
+			cradle = &blanket->next;
+}
+
+int async_execute(int fd, void *data)
+{
+	int ret = execute(fd, data);
+	close(fd);
+	free(data);
+	return ret;
 }
 
 static void handle(int incoming, struct sockaddr *addr, int addrlen)
 {
-	pid_t pid;
+	struct sockaddr_storage *ss;
+	struct async async = { 0 };
 
 	if (max_connections && live_children >= max_connections) {
 		kill_some_child();
@@ -696,22 +693,18 @@ static void handle(int incoming, struct sockaddr *addr, int addrlen)
 		}
 	}
 
-	if ((pid = fork())) {
-		close(incoming);
-		if (pid < 0) {
-			logerror("Couldn't fork %s", strerror(errno));
-			return;
-		}
+	ss = xmalloc(sizeof(*ss));
+	memcpy(ss, addr, addrlen);
 
-		add_child(pid, addr, addrlen);
-		return;
-	}
+	async.proc = async_execute;
+	async.data = ss;
+	async.out = incoming;
 
-	dup2(incoming, 0);
-	dup2(incoming, 1);
+	if (start_async(&async))
+		logerror("unable to fork");
+	else
+		add_child(&async, addr, addrlen);
 	close(incoming);
-
-	exit(execute(0, addr));
 }
 
 static void child_handler(int signo)
-- 
1.6.5.rc2.7.g4f8d3

[PATCH/RFC 10/11] daemon: use full buffered mode for stderr

[Erik Faye-Lund]

Windows doesn't support line buffered mode for file
streams, so let's just use full buffered mode with
a big buffer ("4096 should be enough for everyone")
and add explicit flushing.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 daemon.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/daemon.c b/daemon.c
index 1b0e290..130e951 100644
--- a/daemon.c
+++ b/daemon.c
@@ -66,12 +66,14 @@ static void logreport(int priority, const char *err, va_list params)
 		syslog(priority, "%s", buf);
 	} else {
 		/*
-		 * Since stderr is set to linebuffered mode, the
+		 * Since stderr is set to buffered mode, the
 		 * logging of different processes will not overlap
+		 * unless they overflow the (rather big) buffers.
 		 */
 		fprintf(stderr, "[%"PRIuMAX"] ", (uintmax_t)getpid());
 		vfprintf(stderr, err, params);
 		fputc('\n', stderr);
+		fflush(stderr);
 	}
 }
 
@@ -1094,7 +1096,7 @@ int main(int argc, char **argv)
 		set_die_routine(daemon_die);
 	} else
 		/* avoid splitting a message in the middle */
-		setvbuf(stderr, NULL, _IOLBF, 0);
+		setvbuf(stderr, NULL, _IOFBF, 4096);
 
 	if (inetd_mode && (group_name || user_name))
 		die("--user and --group are incompatible with --inetd");
-- 
1.6.5.rc2.7.g4f8d3

[PATCH/RFC 11/11] mingw: compile git-daemon

[Erik Faye-Lund]

--user and --detach are disabled on Windows due to lack of
fork(), setuid(), setgid(), setsid() and initgroups().

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 Makefile       |    6 +++---
 compat/mingw.h |    1 +
 daemon.c       |   19 ++++++++++++++-----
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 3b01694..406ca81 100644
--- a/Makefile
+++ b/Makefile
@@ -352,6 +352,7 @@ EXTRA_PROGRAMS =
 
 # ... and all the rest that could be moved out of bindir to gitexecdir
 PROGRAMS += $(EXTRA_PROGRAMS)
+PROGRAMS += git-daemon$X
 PROGRAMS += git-fast-import$X
 PROGRAMS += git-hash-object$X
 PROGRAMS += git-imap-send$X
@@ -981,6 +982,8 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
 	NO_REGEX = YesPlease
 	BLK_SHA1 = YesPlease
+	NO_INET_PTON = YesPlease
+	NO_INET_NTOP = YesPlease
 	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/fnmatch
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
 	# We have GCC, so let's make use of those nice options
@@ -1079,9 +1082,6 @@ ifdef ZLIB_PATH
 endif
 EXTLIBS += -lz
 
-ifndef NO_POSIX_ONLY_PROGRAMS
-	PROGRAMS += git-daemon$X
-endif
 ifndef NO_OPENSSL
 	OPENSSL_LIBSSL = -lssl
 	ifdef OPENSSLDIR
diff --git a/compat/mingw.h b/compat/mingw.h
index 576b1a1..1b0dd5b 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -6,6 +6,7 @@
 
 typedef int pid_t;
 typedef int socklen_t;
+typedef unsigned int gid_t;
 #define hstrerror strerror
 
 #define S_IFLNK    0120000 /* Symbolic link */
diff --git a/daemon.c b/daemon.c
index 130e951..5872ec7 100644
--- a/daemon.c
+++ b/daemon.c
@@ -616,7 +616,7 @@ static struct child {
 	struct sockaddr_storage address;
 } *firstborn;
 
-static void add_child(struct async *async, struct sockaddr *addr, int addrlen)
+static void add_child(struct async *async, struct sockaddr *addr, socklen_t addrlen)
 {
 	struct child *newborn, **cradle;
 
@@ -679,7 +679,7 @@ int async_execute(int fd, void *data)
 	return ret;
 }
 
-static void handle(int incoming, struct sockaddr *addr, int addrlen)
+static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
 {
 	struct sockaddr_storage *ss;
 	struct async async = { 0 };
@@ -884,7 +884,7 @@ static int service_loop(int socknum, int *socklist)
 		for (i = 0; i < socknum; i++) {
 			if (pfd[i].revents & POLLIN) {
 				struct sockaddr_storage ss;
-				unsigned int sslen = sizeof(ss);
+				socklen_t sslen = sizeof(ss);
 				int incoming = accept(pfd[i].fd, (struct sockaddr *)&ss, &sslen);
 				if (incoming < 0) {
 					switch (errno) {
@@ -916,6 +916,7 @@ static void sanitize_stdfds(void)
 
 static void daemonize(void)
 {
+#ifndef WIN32
 	switch (fork()) {
 		case 0:
 			break;
@@ -930,6 +931,9 @@ static void daemonize(void)
 	close(1);
 	close(2);
 	sanitize_stdfds();
+#else
+	die("--detach is not supported on Windows");
+#endif
 }
 
 static void store_pid(const char *path)
@@ -950,10 +954,12 @@ static int serve(char *listen_addr, int listen_port, struct passwd *pass, gid_t
 		die("unable to allocate any listen sockets on host %s port %u",
 		    listen_addr, listen_port);
 
+#ifndef WIN32
 	if (pass && gid &&
 	    (initgroups(pass->pw_name, gid) || setgid (gid) ||
 	     setuid(pass->pw_uid)))
 		die("cannot drop privileges");
+#endif
 
 	return service_loop(socknum, socklist);
 }
@@ -966,7 +972,6 @@ int main(int argc, char **argv)
 	const char *pid_file = NULL, *user_name = NULL, *group_name = NULL;
 	int detach = 0;
 	struct passwd *pass = NULL;
-	struct group *group;
 	gid_t gid = 0;
 	int i;
 
@@ -1110,6 +1115,7 @@ int main(int argc, char **argv)
 		die("--group supplied without --user");
 
 	if (user_name) {
+#ifndef WIN32
 		pass = getpwnam(user_name);
 		if (!pass)
 			die("user not found - %s", user_name);
@@ -1117,12 +1123,15 @@ int main(int argc, char **argv)
 		if (!group_name)
 			gid = pass->pw_gid;
 		else {
-			group = getgrnam(group_name);
+			struct group *group = getgrnam(group_name);
 			if (!group)
 				die("group not found - %s", group_name);
 
 			gid = group->gr_gid;
 		}
+#else
+		die("--user is not supported on Windows");
+#endif
 	}
 
 	if (strict_paths && (!ok_paths || !*ok_paths))
-- 
1.6.5.rc2.7.g4f8d3

Re: [PATCH/RFC 00/11] daemon-win32

[Erik Faye-Lund]

Awww, sorry people. I managed to send this one to the wrong
msysgit-account. I'll repost, as I think it's important to get that
list involved as well.

On Thu, Nov 26, 2009 at 1:39 AM, Erik Faye-Lund
<kusmabite@googlemail.com> wrote:
> This is my stab at cleaning up Mike Pape's patches for git-daemon
> on Windows for submission, plus some of my own.
>
> * Patch 1-4 originate from Mike Pape, but have been cleaned up quite
>  a lot by me. I hope Mike won't hate me. Credit have been retained,
>  as all important code here were written by Mike. The commit
>  messages were written mostly by me.
>
> * Patch 5 is a trivial old-style function declaration fix.
>
> * Patch 6 introduces two new functions to the run-command API,
>  kill_async() and is_async_alive().
>
> * Patch 7-8 removes the stdin/stdout redirection for the service
>  functions, as redirecting won't work for the threaded version.
>
> * Patch 9 converts the daemon-code to use the run-command API. This
>  is the patch I expect to be the most controversial.
>
> * Patch 10 is about using line-buffered mode instead of full-buffered
>  mode for stderr.
>
> * Patch 11 finally enables compilation of git-daemon on MinGW.
>
> Let the flames begin!
>
> Erik Faye-Lund (7):
>  inet_ntop: fix a couple of old-style decls
>  run-command: add kill_async() and is_async_alive()
>  run-command: support input-fd
>  daemon: use explicit file descriptor
>  daemon: use run-command api for async serving
>  daemon: use full buffered mode for stderr
>  mingw: compile git-daemon
>
> Mike Pape (4):
>  mingw: add network-wrappers for daemon
>  strbuf: add non-variadic function strbuf_vaddf()
>  mingw: implement syslog
>  compat: add inet_pton and inet_ntop prototypes
>
>  Makefile           |    8 ++-
>  compat/inet_ntop.c |   22 ++------
>  compat/inet_pton.c |    8 ++-
>  compat/mingw.c     |  111 +++++++++++++++++++++++++++++++++++++++++--
>  compat/mingw.h     |   32 ++++++++++++
>  daemon.c           |  134 ++++++++++++++++++++++++++--------------------------
>  git-compat-util.h  |    9 ++++
>  run-command.c      |   32 ++++++++++++-
>  run-command.h      |    2 +
>  strbuf.c           |   15 ++++--
>  strbuf.h           |    1 +
>  11 files changed, 274 insertions(+), 100 deletions(-)
>
>



-- 
Erik "kusma" Faye-Lund

[PATCH/RFC 01/11] mingw: add network-wrappers for daemon

[Erik Faye-Lund]

From: Mike Pape <dotzenlabs@gmail.com>

git-daemon requires some socket-functionality that is not yet
supported in the Windows-port. This patch adds said functionality,
and makes sure WSAStartup gets called by socket(), since it is the
first network-call in git-daemon. In addition, a check is added to
prevent WSAStartup (and WSACleanup, though atexit) from being
called more than once, since git-daemon calls both socket() and
gethostbyname().

Signed-off-by: Mike Pape <dotzenlabs@gmail.com>
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
 compat/mingw.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 compat/mingw.h |   16 ++++++++++++++
 2 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index 10d6796..458021b 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -983,23 +983,37 @@ char **make_augmented_environ(const char *const *vars)
 	return env;
 }
 
-/* this is the first function to call into WS_32; initialize it */
-#undef gethostbyname
-struct hostent *mingw_gethostbyname(const char *host)
+static void wsa_init(void)
 {
+	static int initialized = 0;
 	WSADATA wsa;
 
+	if (initialized)
+		return;
+
 	if (WSAStartup(MAKEWORD(2,2), &wsa))
 		die("unable to initialize winsock subsystem, error %d",
 			WSAGetLastError());
 	atexit((void(*)(void)) WSACleanup);
+	initialized = 1;
+}
+
+/* this can be the first function to call into WS_32; initialize it */
+#undef gethostbyname
+struct hostent *mingw_gethostbyname(const char *host)
+{
+	wsa_init();
 	return gethostbyname(host);
 }
 
+/* this can be the first function to call into WS_32; initialize it */
 int mingw_socket(int domain, int type, int protocol)
 {
+	SOCKET s;
 	int sockfd;
-	SOCKET s = WSASocket(domain, type, protocol, NULL, 0, 0);
+
+	wsa_init();
+	s = WSASocket(domain, type, protocol, NULL, 0, 0);
 	if (s == INVALID_SOCKET) {
 		/*
 		 * WSAGetLastError() values are regular BSD error codes
@@ -1029,6 +1043,44 @@ int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz)
 	return connect(s, sa, sz);
 }
 
+#undef bind
+int mingw_bind(int sockfd, struct sockaddr *sa, size_t sz)
+{
+	SOCKET s = (SOCKET)_get_osfhandle(sockfd);
+	return bind(s, sa, sz);
+}
+
+#undef setsockopt
+int mingw_setsockopt(int sockfd, int lvl, int optname, void *optval, int optlen)
+{
+	SOCKET s = (SOCKET)_get_osfhandle(sockfd);
+	return setsockopt(s, lvl, optname, (const char*)optval, optlen);
+}
+
+#undef listen
+int mingw_listen(int sockfd, int backlog)
+{
+	SOCKET s = (SOCKET)_get_osfhandle(sockfd);
+	return listen(s, backlog);
+}
+
+#undef accept
+int mingw_accept(int sockfd1, struct sockaddr *sa, socklen_t *sz)
+{
+	int sockfd2;
+
+	SOCKET s1 = (SOCKET)_get_osfhandle(sockfd1);
+	SOCKET s2 = accept(s1, sa, sz);
+
+	/* convert into a file descriptor */
+	if ((sockfd2 = _open_osfhandle(s2, O_RDWR|O_BINARY)) < 0) {
+		closesocket(s2);
+		return error("unable to make a socket file descriptor: %s",
+			strerror(errno));
+	}
+	return sockfd2;
+}
+
 #undef rename
 int mingw_rename(const char *pold, const char *pnew)
 {
diff --git a/compat/mingw.h b/compat/mingw.h
index 1978f8a..f362f61 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -5,6 +5,7 @@
  */
 
 typedef int pid_t;
+typedef int socklen_t;
 #define hstrerror strerror
 
 #define S_IFLNK    0120000 /* Symbolic link */
@@ -33,6 +34,9 @@ typedef int pid_t;
 #define F_SETFD 2
 #define FD_CLOEXEC 0x1
 
+#define EAFNOSUPPORT WSAEAFNOSUPPORT
+#define ECONNABORTED WSAECONNABORTED
+
 struct passwd {
 	char *pw_name;
 	char *pw_gecos;
@@ -182,6 +186,18 @@ int mingw_socket(int domain, int type, int protocol);
 int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz);
 #define connect mingw_connect
 
+int mingw_bind(int sockfd, struct sockaddr *sa, size_t sz);
+#define bind mingw_bind
+
+int mingw_setsockopt(int sockfd, int lvl, int optname, void *optval, int optlen);
+#define setsockopt mingw_setsockopt
+
+int mingw_listen(int sockfd, int backlog);
+#define listen mingw_listen
+
+int mingw_accept(int sockfd, struct sockaddr *sa, socklen_t *sz);
+#define accept mingw_accept
+
 int mingw_rename(const char*, const char*);
 #define rename mingw_rename
 
-- 
1.6.5.rc2.7.g4f8d3

Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon

[Martin Storsjö]

Hi,

First of all, great that you're working on adding daemon support for 
windows!

On Thu, 26 Nov 2009, Erik Faye-Lund wrote:

> +static void wsa_init(void)
>  {
> +	static int initialized = 0;
>  	WSADATA wsa;
>  
> +	if (initialized)
> +		return;
> +
>  	if (WSAStartup(MAKEWORD(2,2), &wsa))
>  		die("unable to initialize winsock subsystem, error %d",
>  			WSAGetLastError());
>  	atexit((void(*)(void)) WSACleanup);
> +	initialized = 1;
> +}

Something similar to this was merged into master recently as part of my 
mingw/ipv6 patches, so by rebasing your patch on top of that, this patch 
will probably get a bit smaller.

Also, the getaddrinfo-compatibility wrappers perhaps may need some minor 
updates to handle the use cases needed for setting up listening sockets.

// Martin

Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon

[Erik Faye-Lund]

On Thu, Nov 26, 2009 at 9:24 AM, Martin Storsjö <martin@martin.st> wrote:
> Hi,
>
> First of all, great that you're working on adding daemon support for
> windows!

Thanks. I meant to send this out a couple of weeks ago, but I wasn't
able to find the time until now.

Also, I wouldn't have come this far without going tired of it without
Mike's patches, so some credit should go to him for doing good initial
work!

> On Thu, 26 Nov 2009, Erik Faye-Lund wrote:
>
>> +static void wsa_init(void)
>>  {
>> +     static int initialized = 0;
>>       WSADATA wsa;
>>
>> +     if (initialized)
>> +             return;
>> +
>>       if (WSAStartup(MAKEWORD(2,2), &wsa))
>>               die("unable to initialize winsock subsystem, error %d",
>>                       WSAGetLastError());
>>       atexit((void(*)(void)) WSACleanup);
>> +     initialized = 1;
>> +}
>
> Something similar to this was merged into master recently as part of my
> mingw/ipv6 patches, so by rebasing your patch on top of that, this patch
> will probably get a bit smaller.

Yeah, I saw your patches, and realized that I needed to rebase my work
at some point, but none of the repos I usually pull from seems to
contain the patches yet. Rebasing will be a requirement before this
can be applied for sure.

>
> Also, the getaddrinfo-compatibility wrappers perhaps may need some minor
> updates to handle the use cases needed for setting up listening sockets.

I expect you're referring to IPv6 support in the wrappers this patch
adds? Unfortunately IPv6 isn't something I'm very familiar with, but
I'll give it a go unless someone else provides some patches...

-- 
Erik "kusma" Faye-Lund

Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon

[Martin Storsjö]

On Thu, 26 Nov 2009, Erik Faye-Lund wrote:

> Yeah, I saw your patches, and realized that I needed to rebase my work
> at some point, but none of the repos I usually pull from seems to
> contain the patches yet. Rebasing will be a requirement before this
> can be applied for sure.

Ok, great! I tried applying it on the latest master, and it wasn't too 
much of work.

> > Also, the getaddrinfo-compatibility wrappers perhaps may need some minor
> > updates to handle the use cases needed for setting up listening sockets.
> 
> I expect you're referring to IPv6 support in the wrappers this patch
> adds? Unfortunately IPv6 isn't something I'm very familiar with, but
> I'll give it a go unless someone else provides some patches...

No, sorry for being unclear.

When IPv6 is enabled, name lookups go through getaddrinfo instead of 
gethostbyname. Since getaddrinfo isn't available on Win2k (and switching 
between getaddrinfo/gethostbyname happens at compile time when IPv6 is 
enabled), we have to provide a small getaddrinfo stub, implemented in 
terms of gethostbyname. This currently implements only parts of the 
getaddrinfo interface - enough for the way getaddrinfo was used this far.

git-daemon uses getaddrinfo in a slightly different way (for setting up 
listening sockets), and thus uses parameters that our current getaddrinfo 
stub doesn't support. The patch I sent to this thread a moment ago adds 
support for the way git-daemon uses getaddrinfo.


I tested this patch series on top of the latest master, with IPv6 support, 
and found a slight problem caused by the IPv6 support. If IPv6 isn't 
enabled, git-daemon always listens on one single socket, otherwise it may 
listen on two separate sockets, one for v4 and one for v6.

This causes problems with the mingw poll() replacement, which has a 
special case for polling one single fd - otherwise it tries to use some 
emulation that currently only works for pipes. I didn't try to make any 
proper fix for this though. I tested git-daemon by hacking it to listen on 
only one of the sockets, and that worked well for both v4 and v6.


So, in addition to the getaddrinfo patch I sent, the mingw poll() 
replacement needs some updates to handle polling multiple sockets. Except 
from that, things seem to work, at a quick glance.

// Martin

Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon

[Erik Faye-Lund]

(CC'ing Hannes, as he seems to be the original author of our poll()-emulation)

On Thu, Nov 26, 2009 at 12:03 PM, Martin Storsjö <martin@martin.st> wrote:
> On Thu, 26 Nov 2009, Erik Faye-Lund wrote:
>
>> Yeah, I saw your patches, and realized that I needed to rebase my work
>> at some point, but none of the repos I usually pull from seems to
>> contain the patches yet. Rebasing will be a requirement before this
>> can be applied for sure.
>
> Ok, great! I tried applying it on the latest master, and it wasn't too
> much of work.

I've finally gotten around to doing the same now, thanks for the heads up :)

> This causes problems with the mingw poll() replacement, which has a
> special case for polling one single fd - otherwise it tries to use some
> emulation that currently only works for pipes. I didn't try to make any
> proper fix for this though. I tested git-daemon by hacking it to listen on
> only one of the sockets, and that worked well for both v4 and v6.
>
>
> So, in addition to the getaddrinfo patch I sent, the mingw poll()
> replacement needs some updates to handle polling multiple sockets. Except
> from that, things seem to work, at a quick glance.
>

Thanks a lot for helping out! I'm seeing the same issue here when
running on Vista:
$ ./git-daemon.exe --base-path=/c/Users/Erik/src/ --export-all
error: PeekNamedPipe failed, GetLastError: 1
[3656] Poll failed, resuming: Invalid argument
error: PeekNamedPipe failed, GetLastError: 1
[3656] Poll failed, resuming: Invalid argument
...

If I hack the poll()-emulation to never enter the PeekNamedPipe
code-path seems to make it work with IPv6 here, but this is obviously
not the correct solution as it breaks pipe-polling:
--->8---
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -360,7 +360,7 @@ int poll(struct pollfd *ufds, unsigned int nfds, int timeout
         * input is available and let the actual wait happen when the
         * caller invokes read().
         */
-       if (nfds == 1) {
+       if (1) {
                if (!(ufds[0].events & POLLIN))
                        return errno = EINVAL, error("POLLIN not set");
                ufds[0].revents = POLLIN;
--->8---

I'm not very familiar with poll(), but if I understand the man-pages
correctly it's waiting for events on file descriptors, and is in our
case used to check for incoming connections, right? If so, I see three
possible ways forward: (1) extending our poll()-emulation to handle
multiple sockets, (2) change daemon.c to check one socket at the time,
and (3) using select() instead of poll().

(1) seems like the "correct" but tricky thing to do, (2) like the
"easy" but nasty thing to do. However, (3) strikes me as the least
dangerous thing to do ;)

For (1), there's also a WSAPoll() function in Windows, but I'm not
sure how to figure out if an fd is a socket or a pipe. There's also
WaitForMultipleObjects.

Here's a quick stab at (3). It seems to work fine under Windows. Not
tested on other platforms, though.

--->8---
--- a/daemon.c
+++ b/daemon.c
@@ -812,26 +812,22 @@ static int socksetup(char *listen_addr, int listen_port, i
nt **socklist_p)

 static int service_loop(int socknum, int *socklist)
 {
-       struct pollfd *pfd;
-       int i;
-
-       pfd = xcalloc(socknum, sizeof(struct pollfd));
-
-       for (i = 0; i < socknum; i++) {
-               pfd[i].fd = socklist[i];
-               pfd[i].events = POLLIN;
-       }
-
        signal(SIGCHLD, child_handler);

        for (;;) {
                int i;
+               fd_set fds;
+               struct timeval timeout = { 0, 0 };

                check_dead_children();

-               if (poll(pfd, socknum, -1) < 0) {
+               FD_ZERO(&fds);
+               for (i = 0; i < socknum; i++)
+                       FD_SET(socklist[i], &fds);
+
+               if (select(0, &fds, NULL, NULL, &timeout) > 0) {
                        if (errno != EINTR) {
-                               logerror("Poll failed, resuming: %s",
+                               logerror("select() failed, resuming: %s",
                                      strerror(errno));
                                sleep(1);
                        }
@@ -839,10 +835,10 @@ static int service_loop(int socknum, int *socklist)
                }

                for (i = 0; i < socknum; i++) {
-                       if (pfd[i].revents & POLLIN) {
+                       if (FD_ISSET(socklist[i], &fds)) {
                                struct sockaddr_storage ss;
                                socklen_t sslen = sizeof(ss);
-                               int incoming = accept(pfd[i].fd,
(struct sockaddr *)&ss, &sslen);
+                               int incoming = accept(socklist[i],
(struct sockaddr *)&ss, &sslen);
                                if (incoming < 0) {
                                        switch (errno) {
                                        case EAGAIN:
@@ -854,6 +850,7 @@ static int service_loop(int socknum, int *socklist)
                                        }
                                }
                                handle(incoming, (struct sockaddr *)&ss, sslen);

+                               break;
                        }
                }
        }
--->8---

-- 
Erik "kusma" Faye-Lund

Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon

[Martin Storsjö]

On Wed, 2 Dec 2009, Erik Faye-Lund wrote:

> I'm not very familiar with poll(), but if I understand the man-pages
> correctly it's waiting for events on file descriptors, and is in our
> case used to check for incoming connections, right? If so, I see three
> possible ways forward: (1) extending our poll()-emulation to handle
> multiple sockets, (2) change daemon.c to check one socket at the time,
> and (3) using select() instead of poll().
> 
> (1) seems like the "correct" but tricky thing to do, (2) like the
> "easy" but nasty thing to do. However, (3) strikes me as the least
> dangerous thing to do ;)

Generally, poll is a better API than select, since select is limited to fd 
values up to (about) 1000, depending on the implementation. (This is due 
to the fact that fd_set is a fixed size bit set with a bit for each 
possible fd.)

But since we're only doing select on the handful of sockets that were 
opened initially in the process (so these should receive low numbers), 
this should only be a theoretical concern.

> For (1), there's also a WSAPoll() function in Windows, but I'm not
> sure how to figure out if an fd is a socket or a pipe. There's also
> WaitForMultipleObjects.
> 
> Here's a quick stab at (3). It seems to work fine under Windows. Not
> tested on other platforms, though.

A few comments below, just by reading through this, didn't test it yet:

> --->8---
> --- a/daemon.c
> +++ b/daemon.c
> @@ -812,26 +812,22 @@ static int socksetup(char *listen_addr, int listen_port, i
> nt **socklist_p)
> 
>  static int service_loop(int socknum, int *socklist)
>  {
> -       struct pollfd *pfd;
> -       int i;
> -
> -       pfd = xcalloc(socknum, sizeof(struct pollfd));
> -
> -       for (i = 0; i < socknum; i++) {
> -               pfd[i].fd = socklist[i];
> -               pfd[i].events = POLLIN;
> -       }
> -
>         signal(SIGCHLD, child_handler);
> 
>         for (;;) {
>                 int i;
> +               fd_set fds;
> +               struct timeval timeout = { 0, 0 };
> 
>                 check_dead_children();
> 
> -               if (poll(pfd, socknum, -1) < 0) {
> +               FD_ZERO(&fds);
> +               for (i = 0; i < socknum; i++)
> +                       FD_SET(socklist[i], &fds);
> +
> +               if (select(0, &fds, NULL, NULL, &timeout) > 0) {
>                         if (errno != EINTR) {

The first parameter to select should be max(all fds set in the fd_set) + 
1, this should be trivial enough to determine in the loop above where the 
fd:s are added with FD_SET.

You're calling select() with a zero timeout - I'd guess this chews quite a 
bit of cpu just looping around doing nothing? If the last parameter would 
be set to NULL, it would wait indefinitely, just like the previous poll 
loop did.

> @@ -854,6 +850,7 @@ static int service_loop(int socknum, int *socklist)
>                                         }
>                                 }
>                                 handle(incoming, (struct sockaddr *)&ss, sslen);
> 
> +                               break;

What's this good for?

Other than that, this looks quite good to me, at a first glance.

// Martin

Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon

[Erik Faye-Lund]

On Wed, Dec 2, 2009 at 2:21 PM, Martin Storsjö <martin@martin.st> wrote:
> On Wed, 2 Dec 2009, Erik Faye-Lund wrote:
>
>> I'm not very familiar with poll(), but if I understand the man-pages
>> correctly it's waiting for events on file descriptors, and is in our
>> case used to check for incoming connections, right? If so, I see three
>> possible ways forward: (1) extending our poll()-emulation to handle
>> multiple sockets, (2) change daemon.c to check one socket at the time,
>> and (3) using select() instead of poll().
>>
>> (1) seems like the "correct" but tricky thing to do, (2) like the
>> "easy" but nasty thing to do. However, (3) strikes me as the least
>> dangerous thing to do ;)
>
> Generally, poll is a better API than select, since select is limited to fd
> values up to (about) 1000, depending on the implementation. (This is due
> to the fact that fd_set is a fixed size bit set with a bit for each
> possible fd.)
>
> But since we're only doing select on the handful of sockets that were
> opened initially in the process (so these should receive low numbers),
> this should only be a theoretical concern.
>

Yeah. In our case, I guess it's a maximum of two times the number of
network adapters installed, so I think we should be relatively safe.

>
>> --->8---
>> --- a/daemon.c
>> +++ b/daemon.c
>> @@ -812,26 +812,22 @@ static int socksetup(char *listen_addr, int listen_port, i
>> nt **socklist_p)
>>
>>  static int service_loop(int socknum, int *socklist)
>>  {
>> -       struct pollfd *pfd;
>> -       int i;
>> -
>> -       pfd = xcalloc(socknum, sizeof(struct pollfd));
>> -
>> -       for (i = 0; i < socknum; i++) {
>> -               pfd[i].fd = socklist[i];
>> -               pfd[i].events = POLLIN;
>> -       }
>> -
>>         signal(SIGCHLD, child_handler);
>>
>>         for (;;) {
>>                 int i;
>> +               fd_set fds;
>> +               struct timeval timeout = { 0, 0 };
>>
>>                 check_dead_children();
>>
>> -               if (poll(pfd, socknum, -1) < 0) {
>> +               FD_ZERO(&fds);
>> +               for (i = 0; i < socknum; i++)
>> +                       FD_SET(socklist[i], &fds);
>> +
>> +               if (select(0, &fds, NULL, NULL, &timeout) > 0) {
>>                         if (errno != EINTR) {
>
> The first parameter to select should be max(all fds set in the fd_set) +
> 1, this should be trivial enough to determine in the loop above where the
> fd:s are added with FD_SET.
>

Yeah, thanks for pointing out. I did a little bit of searching, and it
seems like "most other people passes zero, and it just works".
However, we should be nice to systems where it doesn't "just work", so
I'll fix this before sending it out.

> You're calling select() with a zero timeout - I'd guess this chews quite a
> bit of cpu just looping around doing nothing? If the last parameter would
> be set to NULL, it would wait indefinitely, just like the previous poll
> loop did.
>

Ah, yes. That's much nicer!

>> @@ -854,6 +850,7 @@ static int service_loop(int socknum, int *socklist)
>>                                         }
>>                                 }
>>                                 handle(incoming, (struct sockaddr *)&ss, sslen);
>>
>> +                               break;
>
> What's this good for?
>

When I clone git://localhost/some-repo, select() returns a fd-set with
both the IPv4 and IPv6 fds. After accept()'ing the first one, the
second call to accept() hangs. I solved this by accepting only the
first connection I got; the second one should be accepted in the next
round of the service loop (if still available).

-- 
Erik "kusma" Faye-Lund

Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon

[Erik Faye-Lund]

On Wed, Dec 2, 2009 at 2:49 PM, Erik Faye-Lund <kusmabite@googlemail.com> wrote:
> On Wed, Dec 2, 2009 at 2:21 PM, Martin Storsjö <martin@martin.st> wrote:
>> On Wed, 2 Dec 2009, Erik Faye-Lund wrote:
>>> @@ -854,6 +850,7 @@ static int service_loop(int socknum, int *socklist)
>>>                                         }
>>>                                 }
>>>                                 handle(incoming, (struct sockaddr *)&ss, sslen);
>>>
>>> +                               break;
>>
>> What's this good for?
>>
>
> When I clone git://localhost/some-repo, select() returns a fd-set with
> both the IPv4 and IPv6 fds. After accept()'ing the first one, the
> second call to accept() hangs. I solved this by accepting only the
> first connection I got; the second one should be accepted in the next
> round of the service loop (if still available).
>

Actually, it's no good - my code is broken. FD_SET() and FD_ISSET()
needs wrapping to call _get_osfhandle() on the socket. Since this is
not needed on Linux, the code ran just fine there. But on Windows,
select() failed, but due to the following bug, it wasn't picked up:
+               if (select(0, &fds, NULL, NULL, &timeout) > 0) {

changing the comparison around, revealed that select hadn't done
anything so fds wasn't modified at all! Thus, I wrongly interpreted it
as if both sockets had an awaiting connection, making IPv6 connections
work (since they are on the first socket), but IPv4 not.

I've corrected this locally, and I'll include the fixed patch in the next round.

Thanks for pointing out the issue, causing me to second guess my "cure" :)

-- 
Erik "kusma" Faye-Lund

Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon

[Johannes Sixt]

On Mittwoch, 2. Dezember 2009, Erik Faye-Lund wrote:
> I'm not very familiar with poll(), but if I understand the man-pages
> correctly it's waiting for events on file descriptors, and is in our
> case used to check for incoming connections, right? If so, I see three
> possible ways forward: (1) extending our poll()-emulation to handle
> multiple sockets, (2) change daemon.c to check one socket at the time,
> and (3) using select() instead of poll().
>
> (1) seems like the "correct" but tricky thing to do, (2) like the
> "easy" but nasty thing to do. However, (3) strikes me as the least
> dangerous thing to do ;)
>
> For (1), there's also a WSAPoll() function in Windows, but I'm not
> sure how to figure out if an fd is a socket or a pipe. There's also
> WaitForMultipleObjects.

GetFileType() returns FILE_TYPE_PIPE for both pipes and sockets. But once you 
know this, you can use getsockopt(): If it succeeds, it is a socket, and in 
this case, assume that poll() was called from git-daemon, i.e. all polled-for 
fds are sockets and you can select().

-- Hannes