From 0e4342fa947fabde643c1482c70a3a3250756570 Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Tue, 24 Sep 2024 16:17:42 +0100 Subject: Re-work nxt_process_check_pid_status() slightly There has been a long standing clang-analyzer issue in nxt_process_check_pid_status(), well ever since I introduced this function in commit b0e2d9d0a ("Isolation: Switch to fork(2) & unshare(2) on Linux."), It is complaining that there are cases where 'status' could be returned with an undefined or garbage value. Now I'm convinced this can't happen If nxt_process_pipe_timer() returns NXT_OK read(2) the status value If the read(2) failed or if we got NXT_ERROR from nxt_process_pipe_timer(), NXT_OK (0) and NXT_ERROR (-1) are the only possible return values here, then we set status to -1 I don't see a scenario where status is either not set by the read(2) or not set to -1. Now I'm not a fan of initialising variables willy-nilly, however, in this case if we initialise status to -1, then we can simply remove the if (ret <= 0) { check. So it closes this (non-)issue but also provides a little code simplification. NOTE: There is no need to check the return from the read(2) here. We are reading a single byte, we either get it or don't. Signed-off-by: Andrew Clayton --- src/nxt_process.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/nxt_process.c b/src/nxt_process.c index ce2de774..f445f0f5 100644 --- a/src/nxt_process.c +++ b/src/nxt_process.c @@ -370,18 +370,14 @@ nxt_process_pipe_timer(nxt_fd_t fd, short event) static nxt_int_t nxt_process_check_pid_status(const nxt_fd_t *gc_pipe) { - int8_t status; + int8_t status = -1; ssize_t ret; close(gc_pipe[1]); ret = nxt_process_pipe_timer(gc_pipe[0], POLLIN); if (ret == NXT_OK) { - ret = read(gc_pipe[0], &status, sizeof(int8_t)); - } - - if (ret <= 0) { - status = -1; + read(gc_pipe[0], &status, sizeof(int8_t)); } close(gc_pipe[0]); -- cgit