Created attachment 125171 [details] [review]
test-bus: don't write to the syslog

We want to emulate the behaviour of the system bus, but we don't
really want to spam the system log with lots of test messages.

Created attachment 125172 [details] [review]
sysdeps: move _dbus_system_log() into the shared  library

This is in preparation for optionally making _dbus_warn() use it.
dbus-daemon closes its stderr under some circumstances, including
when launched by dbus-launch, which makes failures in that
situation rather hard to debug.

_dbus_system_log() is the same on Unix and Windows, so move it
to dbus-sysdeps.c. _dbus_system_logv() remains platform-specific.

Created attachment 125173 [details] [review]
_dbus_init_system_log: record a syslog tag (executable  name)

Instead of hard-coding "dbus", report what the executable really is.

---

The only API users right now are dbus-daemon and test-syslog, but dbus-daemon-launch-helper (Bug #85106) and dbus-launch could potentially use this too.

Created attachment 125174 [details] [review]
_dbus_system_log: rename to _dbus_log

This is a step towards making it write to either stderr or syslog
or both, as configured globally.

Created attachment 125175 [details] [review]
config-parser: remove unnecessary newline from  _dbus_log()

_dbus_log() already adds a newline.

Created attachment 125176 [details] [review]
_dbus_logv: configurably log to syslog and/or stderr

Created attachment 125177 [details] [review]
bus_context_log: remove special handling of  !context->syslog

_dbus_log() and _dbus_logv() are always the right functions to call now.

Created attachment 125178 [details] [review]
_dbus_warn, _dbus_warn_check_failed: unify with  _dbus_logv

This means that dbus-daemon will log something like

    dbus-daemon[123]: Unable to add reload watch to main loop

to syslog and/or stderr according to its configuration, while other
libdbus users will print something like this to stderr:

    dbus[4567]: arguments to dbus_foo() were incorrect, assertion
    "connection != NULL" failed at file dbus-foo.c line 123.
    This is normally a bug in some application using the D-Bus library.

This slightly changes the meaning of the argument to _dbus_warn()
and _dbus_warn_check_failed. Previously, a trailing newline was
expected, and a missing newline would have resulted in incorrect
output. Now, a newline is supplied automatically by the
library (like g_warning()), and messages that end with a newline will
result in an unnecessary extra newline in output.

This extra newline is harmless, so I'm not going to change all the
callers immediately.

Created attachment 125179 [details] [review]
syslog test: extend assertions to cover new  functionality

Created attachment 125180 [details] [review]
dbus-daemon: add --syslog, --nosyslog, --syslog-only

Like --fork and --nofork, these override what the configuration says.

Use --syslog-only to force the systemd services to log to the Journal
(via syslog, which means we see the severity metadata) instead of
testing sd_booted() in the configuration implementation.

---

With hindsight I think this should always have been a command-line option, and never a config option, like --fork/--nofork.

Other things that could potentially be done here:

* make dbus-launch run dbus-daemon --syslog-only (or --syslog if it didn't close stderr) so that the messages don't just go to /dev/null
* make dbus-launch use syslog itself, if useful
* Bug #85106

This is potentially also useful for Bug #85108 and Bug #86442.

Comment on attachment 125173 [details] [review]
_dbus_init_system_log: record a syslog tag (executable  name)

Review of attachment 125173 [details] [review]:
-----------------------------------------------------------------

::: dbus/dbus-sysdeps-unix.c
@@ +4436,2 @@
>  void
> +_dbus_init_system_log (const char *tag,

I should probably write a doc-comment to clarify that the tag is not strdup()'d, and must be available for the entire lifetime of the process.

(In reply to Simon McVittie from comment #8)
>     dbus[4567]: arguments to dbus_foo() were incorrect, assertion
>     "connection != NULL" failed at file dbus-foo.c line 123.
>     This is normally a bug in some application using the D-Bus library.

I've also wondered whether the default syslog tag should be "libdbus" or "process" as a way to indicate that this is probably a bug in (some other library in) process 4567, not "the D-Bus system".

Or it could even be NULL and have a special case for NULL that attempts to read /proc/self/comm (once) and use that, but then we get into thread-safety and "but what if OOM?" considerations, so I'd rather not do that.

(In reply to Simon McVittie from comment #11)
> Other things that could potentially be done here:

* remove \n from all the _dbus_warn() calls
* remove process IDs, likewise
* optionally (?) include timestamps and thread IDs like we do for
  _dbus_verbose (but I think there's a serious risk of overcomplicating it,
  and I'm trying hard not to introduce new places where we have to check
  for OOM in this patchset)

Created attachment 125190 [details] [review]
test-bus: don't write to the syslog

We want to emulate the behaviour of the system bus, but we don't
really want to spam the system log with lots of test messages.

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125191 [details] [review]
sysdeps: move _dbus_system_log() into the shared library

This is in preparation for optionally making _dbus_warn() use it.
dbus-daemon closes its stderr under some circumstances, including
when launched by dbus-launch, which makes failures in that
situation rather hard to debug.

_dbus_system_log() is the same on Unix and Windows, so move it
to dbus-sysdeps.c. _dbus_system_logv() remains platform-specific.

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125192 [details] [review]
_dbus_init_system_log: record a syslog tag (executable name)

Instead of hard-coding "dbus", report what the executable really is.

Signed-off-by: Simon McVittie <smcv@debian.org>

---

Add doc-comment indicating that the syslog tag must remain valid forever

Created attachment 125193 [details] [review]
_dbus_system_log: rename to _dbus_log

This is a step towards making it write to either stderr or syslog
or both, as configured globally.

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125194 [details] [review]
config-parser: remove unnecessary newline from _dbus_log()

_dbus_log() already adds a newline.

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125195 [details] [review]
_dbus_logv: configurably log to syslog and/or stderr

Signed-off-by: Simon McVittie <smcv@debian.org>

---

Add doc-comment

Created attachment 125196 [details] [review]
bus_context_log: remove special handling of !context->syslog

_dbus_log() and _dbus_logv() are always the right functions to call now.

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125197 [details] [review]
_dbus_warn, _dbus_warn_check_failed: unify with _dbus_logv

This means that dbus-daemon will log something like

    dbus-daemon[123]: Unable to add reload watch to main loop

to syslog and/or stderr according to its configuration, while other
libdbus users will print something like this to stderr:

    dbus[4567]: arguments to dbus_foo() were incorrect, assertion
    "connection != NULL" failed at file dbus-foo.c line 123.
    This is normally a bug in some application using the D-Bus library.

This slightly changes the meaning of the argument to _dbus_warn()
and _dbus_warn_check_failed. Previously, a trailing newline was
expected, and a missing newline would have resulted in incorrect
output. Now, a newline is supplied automatically by the
library (like g_warning()), and messages that end with a newline will
result in an unnecessary extra newline in output.

This extra newline is harmless, so I'm not going to change all the
callers immediately.

Signed-off-by: Simon McVittie <smcv@debian.org>

---

Still the same: I haven't changed the default syslog tag to "libdbus"
or "process" as I wondered about.

Created attachment 125198 [details] [review]
syslog test: extend assertions to cover new functionality

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125199 [details] [review]
dbus-daemon: add --syslog, --nosyslog, --syslog-only

Like --fork and --nofork, these override what the configuration says.

Use --syslog-only to force the systemd services to log to the Journal
(via syslog, which means we see the severity metadata) instead of
testing sd_booted() in the configuration implementation.

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125200 [details] [review]
Only redirect child processes to systemd Journal if using syslog

In particular this means the test suite won't spam the Journal
any more.

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125201 [details] [review]
dbus-launch: redirect dbus-daemon output to syslog

Otherwise, it won't go anywhere at all.

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125202 [details] [review]
_dbus_change_to_daemon_user (audit code path): set DBusError correctly

The other error cases here all set the error instead of just warning.

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125203 [details] [review]
internals: remove newline, pid from assertion failures

_dbus_warn() now calls _dbus_logv() which always logs the pid and
prints a newline anyway.

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125204 [details] [review]
Remove trailing newlines from _dbus_assert_not_reached

This was never necessary: _dbus_assert_not_reached() always added one.

Signed-off-by: Simon McVittie <smcv@debian.org>

Created attachment 125205 [details] [review]
Remove trailing newlines from _dbus_warn, _dbus_warn_check_failed

They used to be needed, but are not needed any more, and we were
never completely consistent about including them in any case.

Signed-off-by: Simon McVittie <smcv@debian.org>

Comment on attachment 125175 [details] [review]
config-parser: remove unnecessary newline from  _dbus_log()

duplicated patch

Comment on attachment 125177 [details] [review]
bus_context_log: remove special handling of  !context->syslog

patch is superseeded

Comment on attachment 125172 [details] [review]
sysdeps: move _dbus_system_log() into the shared  library

superseeded

Comment on attachment 125173 [details] [review]
_dbus_init_system_log: record a syslog tag (executable  name)

superseeded

Comment on attachment 125190 [details] [review]
test-bus: don't write to the syslog

Review of attachment 125190 [details] [review]:
-----------------------------------------------------------------

looks good

Comment on attachment 125191 [details] [review]
sysdeps: move _dbus_system_log() into the shared library

Review of attachment 125191 [details] [review]:
-----------------------------------------------------------------

looks good

Comment on attachment 125192 [details] [review]
_dbus_init_system_log: record a syslog tag (executable name)

Review of attachment 125192 [details] [review]:
-----------------------------------------------------------------

looks good

Comment on attachment 125193 [details] [review]
_dbus_system_log: rename to _dbus_log

Review of attachment 125193 [details] [review]:
-----------------------------------------------------------------

looks good

Comment on attachment 125195 [details] [review]
_dbus_logv: configurably log to syslog and/or stderr

Review of attachment 125195 [details] [review]:
-----------------------------------------------------------------

::: dbus/dbus-sysdeps-unix.c
@@ +4483,4 @@
>  {
>    va_list tmp;
>  #ifdef HAVE_SYSLOG_H
> +  if (log_mode != DBUS_LOG_MODE_STDERR)

Not better to use == operator instead != which is easier to read ?

@@ +4508,5 @@
> +      vsyslog (flags, msg, tmp);
> +      va_end (tmp);
> +    }
> +
> +  if (log_mode != DBUS_LOG_MODE_SYSTEM_LOG_ONLY)

see above

::: dbus/dbus-sysdeps-win.c
@@ +3634,5 @@
>    _dbus_win_set_errno (saved_errno);
>  }
>  
> +static const char *log_tag = "dbus";
> +static DBusLogMode log_mode = DBUS_LOG_MODE_STDERR;

Previous implementation indicates that DBUS_LOG_MODE_SYSTEM_LOG_ONLY need to be used here.

@@ +3681,4 @@
>       case DBUS_SYSTEM_LOG_FATAL: s = "fatal"; break;
>     }
>  
> +  if (log_mode != DBUS_LOG_MODE_STDERR)

Not better to use log_mode == instead of != ?

::: dbus/dbus-sysdeps.h
@@ +556,5 @@
>                                     DBusError  *error);
>  
> +typedef enum {
> +  DBUS_LOG_MODE_STDERR,
> +  DBUS_LOG_MODE_SYSTEM_LOG,

Here should be a note that DBUS_LOG_MODE_SYSTEM_LOG is a combination of DBUS_LOG_MODE_STDERR and BUS_LOG_MODE_SYSTEM_LOG_ONLY.
Alternativly another term could be used to indicate that this mode dumps to system log *and* to stderr.

Comment on attachment 125196 [details] [review]
bus_context_log: remove special handling of !context->syslog

Review of attachment 125196 [details] [review]:
-----------------------------------------------------------------

looks good if the answer to the question below is: yes.

::: bus/bus.c
@@ -1426,5 @@
> -      fputs (msg, stderr);
> -      fputc ('\n', stderr);
> -
> -      if (severity == DBUS_SYSTEM_LOG_FATAL)
> -        _dbus_exit (1);

Does dbus_log has code to exit on DBUS_SYSTEM_LOG_FATAL severity ?

Comment on attachment 125197 [details] [review]
_dbus_warn, _dbus_warn_check_failed: unify with _dbus_logv

Review of attachment 125197 [details] [review]:
-----------------------------------------------------------------

::: dbus/dbus-internals.c
@@ -262,5 @@
>    
>    if (!warn_initted)
>      init_warnings ();
>  
> -  fprintf (stderr, "process %lu: ", _dbus_pid_for_log ());

Does _dbus_logv supports printing pid to system log, which seems to be the default on entries in the linux system log ? Application message starts always with <processname>[<pid>].

Otherwise this information will be lost with this patch.

Comment on attachment 125199 [details] [review]
dbus-daemon: add --syslog, --nosyslog, --syslog-only

Review of attachment 125199 [details] [review]:
-----------------------------------------------------------------

looks good

Comment on attachment 125200 [details] [review]
Only redirect child processes to systemd Journal if using syslog

Review of attachment 125200 [details] [review]:
-----------------------------------------------------------------

looks good

(In reply to Ralf Habacker from comment #39)
> > +  if (log_mode != DBUS_LOG_MODE_STDERR)
> 
> Not better to use == operator instead != which is easier to read ?

IMO no, because it's a tristate. We want to log to syslog, unless the mode is the single mode out of the three that does not log to syslog.

> > +  if (log_mode != DBUS_LOG_MODE_SYSTEM_LOG_ONLY)

Similarly, here we want to log to stderr, unless the mode is the one mode out of the three that does not log to stderr.

> > +static const char *log_tag = "dbus";
> > +static DBusLogMode log_mode = DBUS_LOG_MODE_STDERR;
> 
> Previous implementation indicates that DBUS_LOG_MODE_SYSTEM_LOG_ONLY need to
> be used here.

The previous implementation was designed to only be used by dbus-daemon (or test-syslog), and only after _dbus_init_system_log() was called first. The new implementation is designed to be called without first calling _dbus_init_system_log(), in some cases. So we want the global defaults to be whatever is correct in the cases where _dbus_init_system_log() has not been called.

In particular, if you are using libdbus from some random application (for instance a KDE app that uses QtDBus), nothing will have called _dbus_init_system_log(), and I would expect _dbus_warn() calls in that process to go to stderr as they have traditionally done.

If you want a different default on Windows, we can have a different default on Windows, but I'm fairly sure that the default on Unix should be STDERR.

> Here should be a note that DBUS_LOG_MODE_SYSTEM_LOG is a combination of
> DBUS_LOG_MODE_STDERR and BUS_LOG_MODE_SYSTEM_LOG_ONLY.

That's reasonable. As background, one common way the system log is used on Unix is the way dbus-daemon has traditionally done it: write each message to syslog and stderr, in parallel. We used to do this via the non-standard LOG_PERROR option, or if that isn't supported, by calling vfprintf() in _dbus_system_logv().

(In reply to Simon McVittie from comment #44)
> So we want the global defaults to be
> whatever is correct in the cases where _dbus_init_system_log() has not been
> called.

(because in the cases where _dbus_init_system_log() *has* been called, the default has been overridden and is irrelevant)

(In reply to Ralf Habacker from comment #40)
> Does dbus_log has code to exit on DBUS_SYSTEM_LOG_FATAL severity ?

Yes. (Not directly, but it calls _dbus_logv() which does.)

(In reply to Ralf Habacker from comment #41)
> > -  fprintf (stderr, "process %lu: ", _dbus_pid_for_log ());
> 
> Does _dbus_logv supports printing pid to system log, which seems to be the
> default on entries in the linux system log ? Application message starts
> always with <processname>[<pid>].

tl;dr: yes it's correct.

_dbus_logv() has two code paths: syslog and stderr. LOG_MODE_SYSLOG uses both.

In the syslog code path, we get the pid logged automatically because we passed LOG_PID to openlog(), and we get the syslog tag (in practice "dbus-daemon") because we passed it to openlog(). If nobody has called _dbus_init_system_log(), which is where we call openlog(), then we are in LOG_MODE_STDERR so this code path is irrelevant.

In the stderr code path, we print the syslog tag and the pid directly. If nobody has called _dbus_init_system_log(), we use a default syslog tag.

If you run the tests on a Linux system, you'll see the results of test-syslog in syslog/the Journal, and also on its stderr. I haven't tried it on real Windows, but it works as expected in Wine, and by setting WINEDEBUG=+relay I was able to see it calling OutputDebugString() in only the expected cases.

(In reply to Ralf Habacker from comment #41)
> Application message starts
> always with <processname>[<pid>].

Strictly speaking that's not the process name, it's the "syslog tag" or "ident", which is *conventionally* the process name but can be different (for instance as a way to distinguish between different roles, like the D-Bus system and session buses).

In dbus-daemon versions that don't have these patches, the syslog tag is "dbus"; one side-effect of these patches is to correct it to the more conventional "dbus-daemon".

(In reply to Simon McVittie from comment #44)
> > Not better to use == operator instead != which is easier to read ?
> 
> IMO no, because it's a tristate. We want to log to syslog, unless the mode
> is the single mode out of the three that does not log to syslog.

One advantage of this approach is that it "fails safe". If someone uses an invalid value, or if we add a new DBUS_LOG_MODE_SOMETHING_ELSE but forget to update all references to the log mode, then we get the most conservative implementation - log to all (in this case both) destinations, so the log message is never silently suppressed.

Comment on attachment 125203 [details] [review]
internals: remove newline, pid from assertion failures

Review of attachment 125203 [details] [review]:
-----------------------------------------------------------------

looks good

Comment on attachment 125204 [details] [review]
Remove trailing newlines from _dbus_assert_not_reached

Review of attachment 125204 [details] [review]:
-----------------------------------------------------------------

looks good

Comment on attachment 125205 [details] [review]
Remove trailing newlines from _dbus_warn, _dbus_warn_check_failed

Review of attachment 125205 [details] [review]:
-----------------------------------------------------------------

looks good otherwise.

::: dbus/dbus-marshal-recursive-util.c
@@ +2648,4 @@
>  
>    if (!_DBUS_DOUBLES_BITWISE_EQUAL (v, expected))
>      {
> +      _dbus_warn ("Expected double %g got %g\n bits = 0x%" PRIx64 " vs.\n bits = 0x%" PRIx64 ")",

You did not change it but: the closing ')' indicates that there might be a opening '(' which is not included in the format string. Looks strange.

Comment on attachment 125202 [details] [review]
_dbus_change_to_daemon_user (audit code path): set DBusError correctly

Review of attachment 125202 [details] [review]:
-----------------------------------------------------------------

looks good.

Comment on attachment 125201 [details] [review]
dbus-launch: redirect dbus-daemon output to syslog

Review of attachment 125201 [details] [review]:
-----------------------------------------------------------------

looks good

Comment on attachment 125198 [details] [review]
syslog test: extend assertions to cover new functionality

Review of attachment 125198 [details] [review]:
-----------------------------------------------------------------

looks good

Comment on attachment 125194 [details] [review]
config-parser: remove unnecessary newline from _dbus_log()

Review of attachment 125194 [details] [review]:
-----------------------------------------------------------------

looks good

I did a review of all patches. Unfortunally splinter review does not have a mode to set any "review+" state for patches, which would make it easier to see what is still open in a big patch set. 

Also related splinter review board at https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=97009 is partially incomplete in showing published reviews.

(In reply to Ralf Habacker from comment #39)
> Previous implementation indicates that DBUS_LOG_MODE_SYSTEM_LOG_ONLY need to
> be used here.

I think I've justified why I changed this, hopefully you agree.

> Here should be a note that DBUS_LOG_MODE_SYSTEM_LOG is a combination of
> DBUS_LOG_MODE_STDERR and BUS_LOG_MODE_SYSTEM_LOG_ONLY.
> Alternativly another term could be used to indicate that this mode dumps to
> system log *and* to stderr.

I've changed this to a bitfield DBusLogFlags containing

DBUS_LOG_FLAGS_STDERR
DBUS_LOG_FLAGS_SYSTEM_LOG

together with an assertion that at least one of them is enabled. I think that addresses your concerns about != vs. ==, too.

Comment on attachment 125190 [details] [review]
test-bus: don't write to the syslog

Applied

Comment on attachment 125191 [details] [review]
sysdeps: move _dbus_system_log() into the shared library

Applied (with a missing #include fixed - it was corrected in a later patch)

Comment on attachment 125192 [details] [review]
_dbus_init_system_log: record a syslog tag (executable name)

Applied

Comment on attachment 125193 [details] [review]
_dbus_system_log: rename to _dbus_log

Applied

Comment on attachment 125194 [details] [review]
config-parser: remove unnecessary newline from _dbus_log()

Applied

Comment on attachment 125204 [details] [review]
Remove trailing newlines from _dbus_assert_not_reached

Applied

Created attachment 125741 [details] [review]
_dbus_logv: configurably log to syslog and/or stderr

This changes the behaviour of _dbus_logv() if _dbus_init_system_log() was
not called. Previously, _dbus_logv() would always log to syslog;
additionally, it would log to stderr, unless the process is dbus-daemon
and it was started by systemd. Now, it will log to stderr only,
unless _dbus_init_system_log() was called first.

This is the desired behaviour because when we hook up
_dbus_warn_check_failed() to _dbus_logv() in the next commit, we don't
want typical users of libdbus to start logging their check failures to
syslog - we only want the dbus-daemon to do that.

In practice this is not usually a behaviour change, because there was
only one situation in which we called _dbus_logv() without first calling
_dbus_init_system_log(), namely an error while parsing configuration
files. Initialize the system log "just in time" in that situation
to preserve existing behaviour.

---

* Changed to use flags instead of an enum
* Amended commit message to explain why the behaviour change.
* Fixed an unintended behaviour change in config-parser

Created attachment 125742 [details] [review]
bus_context_log: remove special handling of  !context->syslog

_dbus_log() and _dbus_logv() are always the right functions to call now.

---

Already reviewed, but depends on Attachment #125741 [details]. I'm re-attaching it to get it in the right sequence.

Created attachment 125743 [details] [review]
_dbus_warn, _dbus_warn_check_failed: unify with  _dbus_logv

This means that dbus-daemon will log something like

    dbus-daemon[123]: Unable to add reload watch to main loop

to syslog and/or stderr according to its configuration, while other
libdbus users will print something like this to stderr:

    dbus[4567]: arguments to dbus_foo() were incorrect, assertion
    "connection != NULL" failed at file dbus-foo.c line 123.
    This is normally a bug in some application using the D-Bus library.

This slightly changes the meaning of the argument to _dbus_warn()
and _dbus_warn_check_failed. Previously, a trailing newline was
expected, and a missing newline would have resulted in incorrect
output. Now, a newline is supplied automatically by the
library (like g_warning()), and messages that end with a newline will
result in an unnecessary extra newline in output.

This extra newline is harmless, so I'm not going to change all the
callers immediately.

---

I've lost track of whether you approved this one already.

Created attachment 125744 [details] [review]
syslog test: extend assertions to cover new  functionality

(In reply to Simon McVittie from comment #67)
> syslog test: extend assertions to cover new  functionality

Sorry, should have said:

---

Redone in terms of the new API with flags.

Created attachment 125745 [details] [review]
dbus-daemon: add --syslog, --nosyslog, --syslog-only

Like --fork and --nofork, these override what the configuration says.

Use --syslog-only to force the systemd services to log to the Journal
(via syslog, which means we see the severity metadata) instead of
testing sd_booted() in the configuration implementation.

---

Redone in terms of the new API with flags.

Created attachment 125746 [details] [review]
Only redirect child processes to systemd Journal if  using syslog

In particular this means the test suite won't spam the Journal
any more.

---

Already reviewed, but depends on Attachment #125741 [details]. I'm re-attaching it to get it in the right sequence.

Created attachment 125747 [details] [review]
dbus-launch: redirect dbus-daemon output to syslog

Otherwise, it won't go anywhere at all.

---

Depends on Attachment #125745 [details]. I'm re-attaching it to get it in the right sequence.

Created attachment 125748 [details] [review]
internals: remove newline, pid from assertion failures

_dbus_warn() now calls _dbus_logv() which always logs the pid and
prints a newline anyway.

---

Already reviewed, but depends on Attachment #125741 [details]. I'm re-attaching it to get it in the right sequence.

Created attachment 125749 [details] [review]
Remove trailing newlines from _dbus_warn,  _dbus_warn_check_failed

They used to be needed, but are not needed any more, and we were
never completely consistent about including them in any case.

---

Already reviewed (with one minor criticism which I fixed in a separate patch), but depends on Attachment #125741 [details]. I'm re-attaching it to get it in the right sequence.

Created attachment 125750 [details] [review]
Remove unmatched parenthesis

---

Fix for your criticism of (the previous version of) Attachment #125749 [details].

Comment on attachment 125202 [details] [review]
_dbus_change_to_daemon_user (audit code path): set DBusError correctly

Applied

Created attachment 125751 [details] [review]
_dbus_logv: configurably log to syslog and/or stderr

This changes the behaviour of _dbus_logv() if _dbus_init_system_log() was
not called. Previously, _dbus_logv() would always log to syslog;
additionally, it would log to stderr, unless the process is dbus-daemon
and it was started by systemd. Now, it will log to stderr only,
unless _dbus_init_system_log() was called first.

This is the desired behaviour because when we hook up
_dbus_warn_check_failed() to _dbus_logv() in the next commit, we don't
want typical users of libdbus to start logging their check failures to
syslog - we only want the dbus-daemon to do that.

In practice this is not usually a behaviour change, because there was
only one situation in which we called _dbus_logv() without first calling
_dbus_init_system_log(), namely an error while parsing configuration
files. Initialize the system log "just in time" in that situation
to preserve existing behaviour.

---

Fix a remaining instance of DBusLogMode in Windows code (should be DBusLogFlags), and a Windows process ID logged with %d where %lu was expected.

I'm not going to bother reattaching the rest of the series, but this one needs to come first.

Merged to master for 1.11.6.