From 3291102045b9568cccd924e0ebf5be328531a252 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 17 Aug 2021 20:10:19 -0500 Subject: [PATCH] Refactor deferred_process handling to be more clearly safe The previous layout confused me for a minute as it suggested it was possible for `pipe_next_read` to be moved twice (once in the first conditional block, then again when the deferred process conditional called `continue` - if and only if the deferred process *was* the last process in the job. This patch clarifies that can't be the case. --- src/exec.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/exec.cpp b/src/exec.cpp index aba8d6378..e89be39cb 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -1076,12 +1076,15 @@ bool exec_job(parser_t &parser, const shared_ptr &j, const io_chain_t &bl } pipe_next_read = std::move(pipes->read); proc_pipes.write = std::move(pipes->write); - } - // Save any deferred process for last. - if (p == deferred_process) { - deferred_pipes = std::move(proc_pipes); - continue; + // Save any deferred process for last. By definition, the deferred process can never be + // the last process in the job, so it's safe to nest this in the outer + // `if (!p->is_last_in_job)` block, which makes it clear that `proc_next_read` will + // always be assigned when we `continue` the loop. + if (p == deferred_process) { + deferred_pipes = std::move(proc_pipes); + continue; + } } // Regular process.