From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753348AbdGUHX7 (ORCPT ); Fri, 21 Jul 2017 03:23:59 -0400 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:19510 "EHLO mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752334AbdGUHX5 (ORCPT ); Fri, 21 Jul 2017 03:23:57 -0400 X-IronPort-AV: E=Sophos;i="5.40,388,1496095200"; d="scan'208";a="284349549" Date: Fri, 21 Jul 2017 09:23:54 +0200 (CEST) From: Julia Lawall X-X-Sender: jll@hadrien To: walter harms cc: Dave Jiang , Christophe JAILLET , "Williams, Dan J" , "Koul, Vinod" , "dmaengine@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "kernel-janitors@vger.kernel.org" Subject: Re: [PATCH] dmaengine: ioatdma: Fix error handling path in 'ioat_dma_self_test()' In-Reply-To: <5971AB9D.80608@bfs.de> Message-ID: References: <20170719221648.30253-1-christophe.jaillet@wanadoo.fr> <59705A93.2090305@bfs.de> <5971AB9D.80608@bfs.de> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 21 Jul 2017, walter harms wrote: > > > Am 20.07.2017 18:56, schrieb Dave Jiang: > > > > > > On 07/20/2017 12:24 AM, walter harms wrote: > >> > >> > >> Am 20.07.2017 00:16, schrieb Christophe JAILLET: > >>> If the 'memcmp' fails, free allocated resources as done in all other > >>> error handling paths. > >>> > >>> Signed-off-by: Christophe JAILLET > >>> --- > >>> Please review carefully, this patch looks "too obvious" to me! > >>> --- > >>> drivers/dma/ioat/init.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c > >>> index ed8ed1192775..948fc1f8fb5c 100644 > >>> --- a/drivers/dma/ioat/init.c > >>> +++ b/drivers/dma/ioat/init.c > >>> @@ -390,7 +390,7 @@ static int ioat_dma_self_test(struct ioatdma_device *ioat_dma) > >>> if (memcmp(src, dest, IOAT_TEST_SIZE)) { > >>> dev_err(dev, "Self-test copy failed compare, disabling\n"); > >>> err = -ENODEV; > >>> - goto free_resources; > >>> + goto unmap_dma; > >>> } > >>> > >>> unmap_dma: > >> > >> ^^^^^^^^^^ > >> > >> > >> is the goto needed at all ? > > > > It's not. However, it may be better to stay there if we happen to add > > additional code after the if block later on and guard against mistakes. > > At least IMO. > > > > If you are happy with that ... its not a big problem. The compiler will > eat that goto anyway but it is unusual so be prepared that other people > may send patches. I agree with Walter. julia > > re, > wh > > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >