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

On Donnerstag, 26. November 2009, Erik Faye-Lund wrote:

+void syslog(int priority, const char *fmt, ...) {

Style: Brace goes to next line.

+	struct strbuf msg;
+	va_list va;
+	WORD logtype;
+
+	strbuf_init(&msg, 0);
+	va_start(va, fmt);
+	strbuf_vaddf(&msg, fmt, va);
+	va_end(va);

I would

	const char* arg;
	if (strcmp(fmt, "%s"))
		die("format string of syslog() not implemented")
	va_start(va, fmt);
	arg = va_arg(va, char*);
	va_end(va);

because we have exactly one invocation of syslog(), which passes "%s" as 
format string. Then strbuf_vaddf is not needed. Or even simpler: declare the 
function as

void syslog(int priority, const char *fmt, const char*arg);

+	ReportEventA(ms_eventlog,
+	    logtype,
+	    (WORD)NULL,
+	    (DWORD)NULL,
+	    NULL,
+	    1,
+	    0,
+	    (const char **)&msg.buf,
+	    NULL);

Why do you pass NULL and apply casts? The first one gives a warning.

Did you read this paragraph in the documentation?
http://msdn.microsoft.com/en-us/library/aa363679(VS.85).aspx

"Note that the string that you log cannot contain %n, where n is an integer 
value (for example, %1) because the event viewer treats it as an insertion 
string. ..."

How are the chances that this condition applies to our use of the function?

-- Hannes