Re: [PATCH v3 3/4] kselftest/arm64: Add tests for SVE vector configuration

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1319 bytes --]

On Thu, Jul 29, 2021 at 05:06:42PM +0100, Dave Martin wrote:
> On Thu, Jul 29, 2021 at 04:15:17PM +0100, Mark Brown wrote:

> > +	child = fork();
> > +	if (child == -1) {
> > +		ksft_print_msg("fork() failed: %d (%s)\n",
> > +			       errno, strerror(errno));
> > +		close(pipefd[0]);
> > +		close(pipefd[1]);
> > +		return -1;

> Since nothing reopens pipefd[0] or pipefd[1], you could also follow the
> "goto out" convention and just (re)close both fds at the end, rather
> than having to repeat the close() multiple times.  But it works as-is.

I find that when fork() gets involved that starts to get confusing since
you have multiple contexts/control flows around and working out which
cleanup path goes where is more of the issue.

> > +		if (!WIFEXITED(ret)) {
> > +			ksft_print_msg("child exited abnormally\n");
> > +			close(pipefd[0]);
> > +			return -1;
> > +		}

> The WEXITSTATUS() check could go outside the loop.

OTOH I find that logically it's part of working out what happened with
the child which is what the loop body is doing.  Anyway I changed to the
do/while you suggested.

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]