Re: [PATCH 05/15] run-command tests: use "return", not "exit"

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Oct 7, 2022, 09:24 UTC
Message-ID: <9f3216e5-1397-2d91-19dc-47e8165783a6@dunelm.org.uk>
In-Reply-To: <patch-05.15-4ebbf6207fe-20220930T111343Z-avarab@gmail.com>

Hi Ævar

On 30/09/2022 12:28, Ævar Arnfjörð Bjarmason wrote:

Show 8 quoted lines

> Change the "run-command" test helper to "return" instead of calling
> "exit", see 338abb0f045 (builtins + test helpers: use return instead of exit() in cmd_*, 2021-06-08)
> 
> Because we'd previously gotten past the SANITIZE=leak check by using
> exit() here we need to move to "goto cleanup" pattern. See
> fdc8f79f1f1 (leak tests: run various "test-tool" tests in t00*.sh
> SANITIZE=leak, 2021-10-12) for prior art. for when this code was opted
> into the "linux-leaks" job.

That commit just adds some TEST_PASSES_SANITIZE_LEAK=true lines, it's not nothing to do with "goto cleanup", I don't think we need to reference any prior art, just explain why we need to add the cleanup which you already do.

Show 5 quoted lines

>   	if (!strcmp(argv[1], "run-command-parallel")) {
> -		exit(run_processes_parallel(jobs, parallel_next,
> -					    NULL, NULL, &proc));
> +		run_processes_parallel(jobs, parallel_next, NULL, NULL,
> +				       &proc);

There is no explanation of why it is safe to discard the return value here. The answer is in the next commit message, but needs to be mentioned here as well.

Best Wishes

Phillip