On Fri, Oct 25, 2024 at 01:04:38AM +0200, Stefano Brivio wrote:I thought we could just set errno to 0, do a bunch of stuff, and check that errno didn't change to infer we succeeded. But clang-tidy, starting with LLVM 19, reports: /home/sbrivio/passt/util.c:465:6: error: An undefined value may be read from 'errno' [clang-analyzer-unix.Errno,-warnings-as-errors] 465 | if (errno) | ^ /usr/include/errno.h:38:16: note: expanded from macro 'errno' 38 | # define errno (*__errno_location ()) | ^~~~~~~~~~~~~~~~~~~~~~ /home/sbrivio/passt/util.c:446:6: note: Assuming the condition is false 446 | if (pid == -1) { | ^~~~~~~~~ /home/sbrivio/passt/util.c:446:2: note: Taking false branch 446 | if (pid == -1) { | ^ /home/sbrivio/passt/util.c:451:6: note: Assuming 'pid' is 0 451 | if (pid) { | ^~~ /home/sbrivio/passt/util.c:451:2: note: Taking false branch 451 | if (pid) { | ^ /home/sbrivio/passt/util.c:463:2: note: Assuming that 'close' is successful; 'errno' becomes undefined after the call 463 | close(devnull_fd); | ^~~~~~~~~~~~~~~~~ /home/sbrivio/passt/util.c:465:6: note: An undefined value may be read from 'errno' 465 | if (errno) | ^ /usr/include/errno.h:38:16: note: expanded from macro 'errno' 38 | # define errno (*__errno_location ()) | ^~~~~~~~~~~~~~~~~~~~~~ And the LLVM documentation for the unix.Errno checker, 1.1.8.3 unix.Errno (C), mentions, at: https://clang.llvm.org/docs/analyzer/checkers.html#unix-errno that: The C and POSIX standards often do not define if a standard library function may change value of errno if the call does not fail. Therefore, errno should only be used if it is known from the return value of a function that the call has failed. which is, somewhat surprisingly, the case for close().Ah, yeah.Instead of using errno, check the actual return values of the calls we issue here. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>--- util.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/util.c b/util.c index b719f9a..02e18fb 100644 --- a/util.c +++ b/util.c @@ -453,16 +453,11 @@ int __daemon(int pidfile_fd, int devnull_fd) exit(EXIT_SUCCESS); } - errno = 0; - - setsid(); - - dup2(devnull_fd, STDIN_FILENO); - dup2(devnull_fd, STDOUT_FILENO); - dup2(devnull_fd, STDERR_FILENO); - close(devnull_fd); - - if (errno) + if (setsid() < 0 || + dup2(devnull_fd, STDIN_FILENO) < 0 || + dup2(devnull_fd, STDOUT_FILENO) < 0 || + dup2(devnull_fd, STDERR_FILENO) < 0 || + close(devnull_fd)) exit(EXIT_FAILURE); return 0;-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson