From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Tue, 24 Apr 2018 11:57:38 +0000 (+0200)
Subject: basic/log: do not use global errno in log_*_errno()
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=9850448bd016ea3301c2a226c2299876e31b2924;p=elogind.git

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.

(cherry picked from commit b29f6480eca0550ba65d30fbece8dd4d4bfe666d)
---

diff --git a/src/basic/log.c b/src/basic/log.c
index 5011af392..c72ace0f4 100644
--- a/src/basic/log.c
+++ b/src/basic/log.c
@@ -715,9 +715,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);