From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753497AbdGUHWK (ORCPT ); Fri, 21 Jul 2017 03:22:10 -0400 Received: from mx02-sz.bfs.de ([194.94.69.103]:33592 "EHLO mx02-sz.bfs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753422AbdGUHWI (ORCPT ); Fri, 21 Jul 2017 03:22:08 -0400 Message-ID: <5971AB9D.80608@bfs.de> Date: Fri, 21 Jul 2017 09:22:05 +0200 From: walter harms Reply-To: wharms@bfs.de User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; de; rv:1.9.1.16) Gecko/20101125 SUSE/3.0.11 Thunderbird/3.0.11 MIME-Version: 1.0 To: Dave Jiang CC: 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()' References: <20170719221648.30253-1-christophe.jaillet@wanadoo.fr> <59705A93.2090305@bfs.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. re, wh