From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6958DC4338F for ; Mon, 2 Aug 2021 10:29:10 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 344F860F5A for ; Mon, 2 Aug 2021 10:29:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 344F860F5A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=HrQJuwH0N2qaRmgnnMKKfAsklzIiZ5eRB260FDmk/i8=; b=R94dDdF4I19/Y7 5wZ185ejXoXU6A+R9cteI7MpjsgiJrft2ABtmb8mhOTC9J3KmVbo2l18nmkt+uE0w9t/qngwL+n1b E+C1lmzpK3sFWjshaMrgWBWiQP+2AdBdG5gUTNhL7ljdCHs/0SvkPbRSvwauqJkXZ90s4ZWHdExdl 9h7wkUumsLrN01cPMcUY5Te6DHFh04kvrAiBtAbhuRQ0MD2gKSCMm5dalZGmOPnRizGrsW0eWNeQD JnKFMH+TSvCrSQnhqhw4Q8FO1bPoOq7gORJy2BFYdciSpYJmtQbEHJ6urVzSzjPXSDM0ANzeYfis8 JxToakYHefs1v/wHSkGg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAVA4-00FjS1-MC; Mon, 02 Aug 2021 10:27:08 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAVA0-00FjQ8-JL for linux-arm-kernel@lists.infradead.org; Mon, 02 Aug 2021 10:27:06 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DE8FDD6E; Mon, 2 Aug 2021 03:27:00 -0700 (PDT) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 048C63F70D; Mon, 2 Aug 2021 03:26:59 -0700 (PDT) Date: Mon, 2 Aug 2021 11:25:29 +0100 From: Dave Martin To: Mark Brown Cc: Catalin Marinas , Will Deacon , Shuah Khan , linux-arm-kernel@lists.infradead.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH v3 3/4] kselftest/arm64: Add tests for SVE vector configuration Message-ID: <20210802102517.GA25258@arm.com> References: <20210729151518.46388-1-broonie@kernel.org> <20210729151518.46388-4-broonie@kernel.org> <20210729160642.GP1724@arm.com> <20210729173411.GT4670@sirena.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210729173411.GT4670@sirena.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210802_032704_742354_B9AFC0C6 X-CRM114-Status: GOOD ( 26.05 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Jul 29, 2021 at 06:34:11PM +0100, Mark Brown wrote: > 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. Ack, the other option would be to factor out the child stuff into a separate function, but this doesn't quite seem worth it here. Although the code seemed a bit repetitive, it is at least clear in its current form, so I don't have a strong view. > > > > + 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. That's a reasonable position, but thinking about it a bit more, there's not really any loop at all here. There definitely is an unwaited-for child and we don't pass WHONANG to wait(), so it will either return the child pid, or fail. Without WUNTRACED or similar, the child must terminate to wake up the wait(). So is this just a matter of pid = wait(&ret); if (pid == -1) { /* barf */ } assert(pid == child); if (!WIFEXITED(ret)) { /* barf */ } if (WEXITSTATUS(ret) != 0) { /* barf */ } /* parse child's stdout etc. */ > 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. Hmmm, usually I at least try to do that, but I did seem to leave rather a lot of trailing junk that time. (Working out which context is relevant is not always an exact science, but in this case, it looks like I just forgot.) Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel