[pulseaudio-discuss] [PATCH] Add windbg log target + Fix configure not honoring --without-nls

[Maarten Bosmans]

2011/6/24 Colin Guthrie <gmane at colin.guthr.ie>:
> Hi,
>
> 'Twas brillig, and Fritz Elfert at 23/06/11 21:31 did gyre and gimble:
>> Maarten suggested to hurry as 1.0 is on it's way, so attached are two
>> more patches:
>>
>> 1. 0001-mingw-logtarget-windbg.patch (win32 platform)
>>
>> Adds a new log target named "windbg" for windows builds which replaces
>> the "syslog" target on this platform. It uses OutputDebugString() for
>> logging. This is a core feature in windows which is similar to the
>> kernel's message ringbuffer (i.e.: no log files, log entirely in memory,
>> not persistent)
>> For reading such logs, a tool named dbgview is available at
>> www.sysinternals.com

This works as advertised, so the functionality of the patch is OK.

> Sounds worthwhile, but I'm not overly keen on all the #ifdefs needed for
> this.
>
> Would it be too much to expect that the term (in e.g. command line and
> config options etc.) remain as "syslog" but on windows this means
> windbg? That would keep the code a lot cleaner at the expense of
> (perhaps) a little loss on clarity for win32 users. Obviously comments
> can be added (unconditionally) to man pages and documentation, to
> alleviate this.

Well, windbg is not actually 1:1 comparable to syslog, as it does not
provide a persistent log of the events, as Fritz said earlier, it is
more comparable to the kernel log ringbuffer.

> As an alernative, I'd be happy enough changing "syslog" to be "system"
> (although still allowing syslog as an undocumented alias) in all the
> code, config and man pages.

This is a good suggestion. But please let the manpage state clearly
that system means syslog. It is not really needed to include windbg in
the manpage, as those are probably not read on Windows.

> I think this would be cleaner overall and save on several #ifdefs.

Not necessarily lesser ifdefs, but might this be cleaner?

           "      --log-target={auto,"
#ifdef OS_IS_WIN32
                               "windbg,"
#endif
#ifdef HAVE_SYSLOG
                               "syslog,"
#endif
                               "stderr,file:PATH}\n"
           "                                        Specify the log target\n"

> If you could fix this and supply a proper git-formatted patch (not just
> a diff) I'll happily commit.

Maarten