Re: [PATCH v3 02/14] mingw: implement syslog

On Sun, Oct 10, 2010 at 11:28 PM, Eric Sunshine[off-list ref]  wrote:

quoted

On 10/10/2010 4:37 PM, Erik Faye-Lund wrote:

quoted

This is the result of the feed-back in v1, where we tried to implement
all format strings. But that turned out to be very complex (due to the
lack of a portable va_copy()) and since we control all call-sites for
syslog and already only use "%s" as the format, it should be OK.

Do you mean vsnprintf() rather than va_copy()?

The problem was lack of portable va_copy, because I tried to add a
non-variadic version of strbuf_addf(), namely strbuf_vaddf() to do the
work.

I guess it could be implemented pretty easily with vsnprintf(),
though. I was afraid of doing that originally because I know there's
portability issues with the return value of snprintf. Luckily it seems
that we have a fix for that in compat/sprintf.c, and we rely on the
return value being correct in strbuf_addf() so it would probably be
safe.

Something like this (on top)

diff --git a/compat/mingw.c b/compat/mingw.c
index bbe45d0..e3f3f92 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c

@@ -1435,17 +1435,24 @@ void openlog(const char *ident, int logopt,

int facility)
  		warning("RegisterEventSource() failed: %lu", GetLastError());
  }

-void syslog(int priority, const char *fmt, const char *arg)
+void syslog(int priority, const char *fmt, ...)
  {
  	WORD logtype;
+	char *str;
+	int str_len;
+	va_list ap;

  	if (!ms_eventlog)
  		return;

-	if (strcmp(fmt, "%s")) {
-		warning("format string of syslog() not implemented");
-		return;
-	}
+	va_start(ap, fmt);
+	str_len = vsnprintf(NULL, 0, fmt, ap);
+	va_end(ap);

+
+	str = malloc(str_len + 1);
+	va_start(ap, fmt);
+	vsnprintf(str, str_len, fmt, ap);
+	va_end(ap);

  	switch (priority) {
  	case LOG_EMERG:

@@ -1478,8 +1485,9 @@ void syslog(int priority, const char *fmt, const

char *arg)
  	    NULL,
  	    1,
  	    0,
-	    (const char **)&arg,
+	    (const char **)&str,
  	    NULL);
+	free(str);
  }