On Wed, Nov 24, 2021 at 02:55:20AM +0100, Ævar Arnfjörð Bjarmason wrote: > I do wonder re [2] and [3] if a simpler and self-contained/isolated > patch in this area might not be a mirage of sorts. I.e. to know whether > the approach in [2] and [3] is safe we basically have to reason about > all the callers of this API anyway, which is what my larger series does. After thinking on the various solutions, the original snippet I posted to just re-initialize the struct in each run seems like the best fit[1]. It's true that this "args/argv" thing is the source of the actual segfault, so any cleanup changes there would address that. But the root of the confusion in setup_pager() is that it inits the child only once, but then uses it multiple times. There could be similar confusion over other fields in the struct (though I don't think there is currently). So this seems like the most direct fix, and applies regardless of any args/argv cleanup. It would become moot if we start to die() on pager setup, which I'd be in favor of. But I think we should fix the segfault bug first, which allows us to worry about the larger behavior change separately. -Peff [1] https://lore.kernel.org/git/YZhVA8DOjHu90gzs@xxxxxxxxxxxxxxxxxxxxxxx/
