basic/log: do not use global errno in log_*_errno()

Quoting https://github.com/systemd/systemd/pull/8760#discussion_r183321060:

> When we originally added the errno patching we went for a "best of both
> worlds" approach, i.e. that we override errno if an error is specified, but
> if no error is specified (i.e. 0 is passed as error code) then we use the
> previously set errno, similar in style how plain `printf()` would do it. In
> retrospect I think we almost never purposefully made use of the second,
> i.e. the plain `printf()` logic, but we multiple times ran into this case
> accidentally and introduced a bug. Hence yes, it probably makes sense to
> switch this over, and consistently ignore the `errno` already set and always
> override it with the error passed in. The only problem I see with that is: I
> wonder if there might be a case or two lurking somewhere where we actually
> made use of the "best of both worlds" approach, and if so, if we can detect
> where... (But then again, even if there is, and we fail to find those cases,
> maybe that's not all bad, as it's just a few new bugs against probably fixing
> many more old and future bugs, if you follow what I mean).

I scanned our codebase, and found some bugs in the value passed to log_*_errno,
but no intentional cases of error=0 being passed.

diff --git a/src/basic/log.c b/src/basic/log.c
index 9e6de2f96d8e0f0019d4b3eed89a31f888e1fef7..bab61d3140ff245858f97bd877e3d9481c16576c 100644 (file)
--- a/src/basic/log.c
+++ b/src/basic/log.c
@@ -686,9 +686,8 @@ int log_internalv_realm(
         if (_likely_(LOG_PRI(level) > log_max_level[realm]))
                 return -error;
 
-        /* Make sure that %m maps to the specified error */
-        if (error != 0)
-                errno = error;
+        /* Make sure that %m maps to the specified error (or "Success"). */
+        errno = error;
 
         (void) vsnprintf(buffer, sizeof buffer, format, ap);