Re: [PATCH bpf-next v4 1/3] selftests/bpf: Clean up call sites of stdio_restore()

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2025-03-04 at 15:39 -0800, Amery Hung wrote:
> On Tue, Mar 4, 2025 at 1:48 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Tue, 2025-03-04 at 08:36 -0800, Amery Hung wrote:
> > > There is no need to call a bunch of stdio_restore() in test_progs if the
> > > scope of stdio redirection is reduced to what it needs to be: only
> > > hijacking tests/subtests' stdio.
> > > 
> > > Also remove an unnecessary check of env.stdout_saved in the crash handler.
> > > 
> > > Signed-off-by: Amery Hung <ameryhung@gmail.com>
> > > ---
> > 
> > If anyone else would look at this commit, here is an alternative
> > description:
> > - functions reset_affinity() and restore_netns() are only called from
> >   run_one_test();
> > - beside other places stdio_restore() is called from reset_affinity(),
> >   restore_netns() and run_one_test() itself;
> > - this commit moves stdio_restore() call in run_one_test() so that
> >   it executes before reset_affinity() and restore_netns().
> > 
> > Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> > 
> 
> I can improve the commit message in the next respin.

If there would be a respin, otherwise no need to worry.

[...]

> > > @@ -1943,6 +1938,9 @@ int main(int argc, char **argv)
> > > 
> > >       sigaction(SIGSEGV, &sigact, NULL);
> > > 
> > > +     env.stdout_saved = stdout;
> > > +     env.stderr_saved = stderr;
> > > +
> > 
> > Nit: why moving these?
> 
> If we assign env.stdout_saved at the very beginning, crash_handler()
> can just call stdio_restore() without checking if env.stdout_saved is
> set or not.

Understood, thank you.

[...]