Am 13.08.20 um 07:17 schrieb Christian Couder: > On Wed, Aug 12, 2020 at 6:54 PM René Scharfe <l.s.r@xxxxxx> wrote: > >> - close(cmd->in); >> + if (ferror(cmd_in) || fflush(cmd_in)) >> + goto error; >> + fclose(cmd_in); >> cmd->in = -1; > > I wonder if setting cmd->in to -1 is still useful... The patch doesn't change its usefulness. It probably was not necessary to begin with. > >> sigchain_pop(SIGPIPE); >> The next line right here (not shown in the diff) returns 0. >> @@ -660,8 +658,8 @@ static int do_reachable_revlist(struct child_process *cmd, >> error: >> sigchain_pop(SIGPIPE); >> >> - if (cmd->in >= 0) >> - close(cmd->in); >> + if (cmd_in) >> + fclose(cmd_in); > > ...as we don't check cmd->in anymore at the end of the function, but > we now check cmd_in instead. So should cmd_in have been set to -1 > instead of cmd->in? This error handler is not reached from the place that sets cmd->in back to -1. It can be reached from a place where cmd_in is still NULL, so this check is necessary and setting cmd_in to NULL above is not needed. René
