On Tue, Nov 30, 2021 at 09:54:33PM +0100, Ævar Arnfjörð Bjarmason wrote: > > * em/missing-pager (2021-11-24) 1 commit > > - pager: fix crash when pager program doesn't exist > > As noted in [2] I'm happy to get this more isolated fix first. I'd > missed that there was a re-submission[3] until now (since In-Reply-To > wasn't maintained). > > The code change in [3] isn't needed anymore when combined with my > ab/run-command. > > Depending on how you're planning to advance these perhaps you'd like to > revert that as it's merged with ab/run-command, or I can re-roll > ab/run-command on top of it if you'd like. > > We could also just leave that now-redundant child_process_init() in > pager.c, but having something that'll amount to cargo-culting to get > around a bug in dead code doesn't seem ideal. It would be nice to have > the API use reflect "argv" and "env" being gone. IMHO that extra init is still doing the right thing, because it means the struct is in the same known state in every run of setup_pager(). Fixing the argv/env thing stops the immediate bug, but there are other potential ones. E.g., starting the command will overwrite the in/out/err parameters; we're OK in this instance because we unconditionally overwrite them. So I think reusing a child_process struct without reinitializing it is always a bit iffy, though it _usually_ works in practice. We should model good use of the API by initializing on each use. -Peff
