On Tue, Mar 4, 2025 at 1:48 PM Eduard Zingerman <eddyz87@xxxxxxxxx> 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@xxxxxxxxx>
> > ---
>
> 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@xxxxxxxxx>
>
I can improve the commit message in the next respin.
Thanks,
Amery
> [...]
>
> > @@ -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.
>
> > env.secs_till_notify = 10;
> > env.secs_till_kill = 120;
> > err = argp_parse(&argp, argc, argv, 0, NULL, &env);
> > @@ -1969,9 +1967,6 @@ int main(int argc, char **argv)
> > return -1;
> > }
> >
> > - env.stdout_saved = stdout;
> > - env.stderr_saved = stderr;
> > -
> > env.has_testmod = true;
> > if (!env.list_test_names) {
> > /* ensure previous instance of the module is unloaded */
>
>
