chiark / gitweb /
basic/log: do not use global errno in log_*_errno()
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Tue, 24 Apr 2018 11:57:38 +0000 (13:57 +0200)
committerSven Eden <yamakuzure@gmx.net>
Fri, 24 Aug 2018 14:47:08 +0000 (16:47 +0200)
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.

src/basic/log.c

index 5ac6528675e20fb938c48a733da473d40b1b5c6e..ad9d49ba92bcf19da0f278ca9e445346dca959c9 100644 (file)
@@ -702,9 +702,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);