From ee7d0b62a716201abc818eb0d1df4c6bb1051336 Mon Sep 17 00:00:00 2001 From: Stefano Brivio Date: Fri, 25 Oct 2024 00:57:58 +0200 Subject: util: Don't use errno after a successful call in __daemon() 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(). Instead of using errno, check the actual return values of the calls we issue here. Signed-off-by: Stefano Brivio Reviewed-by: David Gibson --- util.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/util.c b/util.c index 1d6d009..dddef93 100644 --- a/util.c +++ b/util.c @@ -444,16 +444,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; -- cgit v1.2.3