Re: [msysGit] [PATCH/RFC 03/11] mingw: implement syslog
- From
Erik Faye-Lund <kusmabite@googlemail.com>
- Date
- Nov 27, 2009, 08:09 UTC
- Message-ID
- <40aa078e0911270009u7569cfe5gb250092c8d2c0eac@mail.gmail.com>
- In-Reply-To
- <200911262223.22777.j6t@kdbg.org>
On Thu, Nov 26, 2009 at 10:23 PM, Johannes Sixt <j6t@kdbg.org> wrote:
Show 24 quoted lines
> 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.
Show 11 quoted lines
>> + 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.
Show 10 quoted lines
> > 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