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

On Thu, Nov 26, 2009 at 10:23 PM, Johannes Sixt [off-list ref] wrote:

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

quoted

+     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);

After reading this again, I agree that this is the best solution. I'll
update for the next iteration.

quoted

+     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.

I'll remove the casts for the next iteration.

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?

Ugh, increasingly high since we're adding IPv6 support, I guess.
Perhaps some form of escaping needs to be done?

-- 
Erik "kusma" Faye-Lund