On Wed, Jun 26, 2024 at 04:34:17PM -0400, Jeff King wrote: > > * What is this thing trying to do? When run_command() fails, it > > wants to leave cp.out open, so that the caller this returns to > > can write into it??? That cannot be the case, as cp itself is > > internal. So does this "close(cp.out)" really matter? > > I think it's totally broken. Using cp.out, cp.in, etc, with > run_command() is a deadlock waiting to happen, since it implies opening > a pipe, not doing anything with our end, and then doing a waitpid() on > the child. > > You'd always want to use start_command(), and then do something useful > with the pipe, and then finish_command(). Arguably run_command() should > bug if cp.out, etc are non-zero. Oh, I guess I should have looked at the code. We do indeed BUG() in this case already, courtesy of c29b3962af (run-command: forbid using run_command with piped output, 2015-03-22). So all is well there. This caller is still completely wrong to call close() though. -Peff
