From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753152AbbDBPSf (ORCPT ); Thu, 2 Apr 2015 11:18:35 -0400 Received: from a.ns.miles-group.at ([95.130.255.143]:65275 "EHLO radon.swed.at" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752295AbbDBPSd (ORCPT ); Thu, 2 Apr 2015 11:18:33 -0400 Message-ID: <551D5DC2.4090209@nod.at> Date: Thu, 02 Apr 2015 17:18:26 +0200 From: Richard Weinberger User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Fabio Estevam CC: Brian Norris , "linux-mtd@lists.infradead.org" , David Woodhouse , linux-kernel Subject: Re: [PATCH] mtd: Add simple read disturb test References: <1427984026-31100-1-git-send-email-richard@nod.at> 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 02.04.2015 um 17:02 schrieb Fabio Estevam: > On Thu, Apr 2, 2015 at 11:13 AM, Richard Weinberger wrote: > >> + ret = mtdtest_erase_eraseblock(mtd, i); >> + if (ret) { >> + err = ret; >> + goto out; >> + } > > Why not just do like this instead? > > err = mtdtest_erase_eraseblock(mtd, i); > if (err) > goto out; > >> + >> + ret = mtdtest_write(mtd, i * mtd->erasesize, mtd->erasesize, >> + iobuf_orig); >> + if (ret) { >> + err = ret; >> + goto out; >> + } > > Same here. The real question is why did I use ret and err at all? ;) This test is based on existing tests, thus it got copy&pasted. I'll think about merging these two variables. Thank for pointing this out. >> + ret = mtdtest_relax(); >> + if (ret) >> + goto out; > > Here you propagate the wrong error. You test for 'ret' and propagate 'err'. This is by design. I don't want to print an error message if the test is aborted. mtdtest_relax() checks for that. Thanks, //richard From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from a.ns.miles-group.at ([95.130.255.143] helo=radon.swed.at) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YdgtS-0005D7-MS for linux-mtd@lists.infradead.org; Thu, 02 Apr 2015 15:18:55 +0000 Message-ID: <551D5DC2.4090209@nod.at> Date: Thu, 02 Apr 2015 17:18:26 +0200 From: Richard Weinberger MIME-Version: 1.0 To: Fabio Estevam Subject: Re: [PATCH] mtd: Add simple read disturb test References: <1427984026-31100-1-git-send-email-richard@nod.at> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: David Woodhouse , Brian Norris , "linux-mtd@lists.infradead.org" , linux-kernel List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Am 02.04.2015 um 17:02 schrieb Fabio Estevam: > On Thu, Apr 2, 2015 at 11:13 AM, Richard Weinberger wrote: > >> + ret = mtdtest_erase_eraseblock(mtd, i); >> + if (ret) { >> + err = ret; >> + goto out; >> + } > > Why not just do like this instead? > > err = mtdtest_erase_eraseblock(mtd, i); > if (err) > goto out; > >> + >> + ret = mtdtest_write(mtd, i * mtd->erasesize, mtd->erasesize, >> + iobuf_orig); >> + if (ret) { >> + err = ret; >> + goto out; >> + } > > Same here. The real question is why did I use ret and err at all? ;) This test is based on existing tests, thus it got copy&pasted. I'll think about merging these two variables. Thank for pointing this out. >> + ret = mtdtest_relax(); >> + if (ret) >> + goto out; > > Here you propagate the wrong error. You test for 'ret' and propagate 'err'. This is by design. I don't want to print an error message if the test is aborted. mtdtest_relax() checks for that. Thanks, //richard