Skip to content

Commit 0e4342f

Browse files
committed
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 b0e2d9d ("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 <a.clayton@nginx.com>
1 parent ac90254 commit 0e4342f

File tree

1 file changed

+2
-6
lines changed

1 file changed

+2
-6
lines changed

src/nxt_process.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -370,18 +370,14 @@ nxt_process_pipe_timer(nxt_fd_t fd, short event)
370370
static nxt_int_t
371371
nxt_process_check_pid_status(const nxt_fd_t *gc_pipe)
372372
{
373-
int8_t status;
373+
int8_t status = -1;
374374
ssize_t ret;
375375

376376
close(gc_pipe[1]);
377377

378378
ret = nxt_process_pipe_timer(gc_pipe[0], POLLIN);
379379
if (ret == NXT_OK) {
380-
ret = read(gc_pipe[0], &status, sizeof(int8_t));
381-
}
382-
383-
if (ret <= 0) {
384-
status = -1;
380+
read(gc_pipe[0], &status, sizeof(int8_t));
385381
}
386382

387383
close(gc_pipe[0]);

0 commit comments

Comments
 (0)