On Thu, Nov 26, 2009 at 10:23 PM, Johannes Sixt <j6t@xxxxxxxx> wrote:
> On Donnerstag, 26. November 2009, Erik Faye-Lund wrote:
>> + 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.
>> + 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html