Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] mtd: no need to check return value of debugfs_create functions
@ 2019-11-07  8:51 Greg Kroah-Hartman
  2019-11-07  9:09 ` Miquel Raynal
  2019-11-12 14:19 ` Miquel Raynal
  0 siblings, 2 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2019-11-07  8:51 UTC (permalink / raw)
  To: David Woodhouse, Brian Norris, Marek Vasut, Miquel Raynal,
	Richard Weinberger, Vignesh Raghavendra, Artem Bityutskiy
  Cc: linux-mtd, linux-kernel

When calling debugfs functions, there is no need to ever check the
return value.  The function can work or not, but the code logic should
never do something different based on this.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Marek Vasut <marek.vasut@gmail.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>
Cc: linux-mtd@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/mtd/mtdcore.c   |  26 +++-----
 drivers/mtd/mtdswap.c   |   8 +--
 drivers/mtd/ubi/debug.c | 128 +++++++++++++---------------------------
 3 files changed, 49 insertions(+), 113 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 6cc7ecb0c788..5fac4355b9c2 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -382,33 +382,21 @@ static struct dentry *dfs_dir_mtd;
 static void mtd_debugfs_populate(struct mtd_info *mtd)
 {
 	struct device *dev = &mtd->dev;
-	struct dentry *root, *dent;
+	struct dentry *root;
 
 	if (IS_ERR_OR_NULL(dfs_dir_mtd))
 		return;
 
 	root = debugfs_create_dir(dev_name(dev), dfs_dir_mtd);
-	if (IS_ERR_OR_NULL(root)) {
-		dev_dbg(dev, "won't show data in debugfs\n");
-		return;
-	}
-
 	mtd->dbg.dfs_dir = root;
 
-	if (mtd->dbg.partid) {
-		dent = debugfs_create_file("partid", 0400, root, mtd,
-					   &mtd_partid_debug_fops);
-		if (IS_ERR_OR_NULL(dent))
-			dev_err(dev, "can't create debugfs entry for partid\n");
-	}
+	if (mtd->dbg.partid)
+		debugfs_create_file("partid", 0400, root, mtd,
+				    &mtd_partid_debug_fops);
 
-	if (mtd->dbg.partname) {
-		dent = debugfs_create_file("partname", 0400, root, mtd,
-					   &mtd_partname_debug_fops);
-		if (IS_ERR_OR_NULL(dent))
-			dev_err(dev,
-				"can't create debugfs entry for partname\n");
-	}
+	if (mtd->dbg.partname)
+		debugfs_create_file("partname", 0400, root, mtd,
+				    &mtd_partname_debug_fops);
 }
 
 #ifndef CONFIG_MMU
diff --git a/drivers/mtd/mtdswap.c b/drivers/mtd/mtdswap.c
index f92414eb4c86..58eefa43af14 100644
--- a/drivers/mtd/mtdswap.c
+++ b/drivers/mtd/mtdswap.c
@@ -1257,7 +1257,6 @@ DEFINE_SHOW_ATTRIBUTE(mtdswap);
 static int mtdswap_add_debugfs(struct mtdswap_dev *d)
 {
 	struct dentry *root = d->mtd->dbg.dfs_dir;
-	struct dentry *dent;
 
 	if (!IS_ENABLED(CONFIG_DEBUG_FS))
 		return 0;
@@ -1265,12 +1264,7 @@ static int mtdswap_add_debugfs(struct mtdswap_dev *d)
 	if (IS_ERR_OR_NULL(root))
 		return -1;
 
-	dent = debugfs_create_file("mtdswap_stats", S_IRUSR, root, d,
-				&mtdswap_fops);
-	if (!dent) {
-		dev_err(d->dev, "debugfs_create_file failed\n");
-		return -1;
-	}
+	debugfs_create_file("mtdswap_stats", S_IRUSR, root, d, &mtdswap_fops);
 
 	return 0;
 }
diff --git a/drivers/mtd/ubi/debug.c b/drivers/mtd/ubi/debug.c
index a1dff92ceedf..bdf65646f092 100644
--- a/drivers/mtd/ubi/debug.c
+++ b/drivers/mtd/ubi/debug.c
@@ -511,8 +511,6 @@ int ubi_debugfs_init_dev(struct ubi_device *ubi)
 {
 	int err, n;
 	unsigned long ubi_num = ubi->ubi_num;
-	const char *fname;
-	struct dentry *dent;
 	struct ubi_debug_info *d = &ubi->dbg;
 
 	if (!IS_ENABLED(CONFIG_DEBUG_FS))
@@ -522,95 +520,51 @@ int ubi_debugfs_init_dev(struct ubi_device *ubi)
 		     ubi->ubi_num);
 	if (n == UBI_DFS_DIR_LEN) {
 		/* The array size is too small */
-		fname = UBI_DFS_DIR_NAME;
-		dent = ERR_PTR(-EINVAL);
-		goto out;
+		return -EINVAL;
 	}
 
-	fname = d->dfs_dir_name;
-	dent = debugfs_create_dir(fname, dfs_rootdir);
-	if (IS_ERR_OR_NULL(dent))
-		goto out;
-	d->dfs_dir = dent;
-
-	fname = "chk_gen";
-	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, (void *)ubi_num,
-				   &dfs_fops);
-	if (IS_ERR_OR_NULL(dent))
-		goto out_remove;
-	d->dfs_chk_gen = dent;
-
-	fname = "chk_io";
-	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, (void *)ubi_num,
-				   &dfs_fops);
-	if (IS_ERR_OR_NULL(dent))
-		goto out_remove;
-	d->dfs_chk_io = dent;
-
-	fname = "chk_fastmap";
-	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, (void *)ubi_num,
-				   &dfs_fops);
-	if (IS_ERR_OR_NULL(dent))
-		goto out_remove;
-	d->dfs_chk_fastmap = dent;
-
-	fname = "tst_disable_bgt";
-	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, (void *)ubi_num,
-				   &dfs_fops);
-	if (IS_ERR_OR_NULL(dent))
-		goto out_remove;
-	d->dfs_disable_bgt = dent;
-
-	fname = "tst_emulate_bitflips";
-	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, (void *)ubi_num,
-				   &dfs_fops);
-	if (IS_ERR_OR_NULL(dent))
-		goto out_remove;
-	d->dfs_emulate_bitflips = dent;
-
-	fname = "tst_emulate_io_failures";
-	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, (void *)ubi_num,
-				   &dfs_fops);
-	if (IS_ERR_OR_NULL(dent))
-		goto out_remove;
-	d->dfs_emulate_io_failures = dent;
-
-	fname = "tst_emulate_power_cut";
-	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, (void *)ubi_num,
-				   &dfs_fops);
-	if (IS_ERR_OR_NULL(dent))
-		goto out_remove;
-	d->dfs_emulate_power_cut = dent;
-
-	fname = "tst_emulate_power_cut_min";
-	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, (void *)ubi_num,
-				   &dfs_fops);
-	if (IS_ERR_OR_NULL(dent))
-		goto out_remove;
-	d->dfs_power_cut_min = dent;
-
-	fname = "tst_emulate_power_cut_max";
-	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, (void *)ubi_num,
-				   &dfs_fops);
-	if (IS_ERR_OR_NULL(dent))
-		goto out_remove;
-	d->dfs_power_cut_max = dent;
-
-	fname = "detailed_erase_block_info";
-	dent = debugfs_create_file(fname, S_IRUSR, d->dfs_dir, (void *)ubi_num,
-				   &eraseblk_count_fops);
-	if (IS_ERR_OR_NULL(dent))
-		goto out_remove;
+	d->dfs_dir = debugfs_create_dir(d->dfs_dir_name, dfs_rootdir);
 
-	return 0;
+	d->dfs_chk_gen = debugfs_create_file("chk_gen", S_IWUSR, d->dfs_dir,
+					     (void *)ubi_num, &dfs_fops);
 
-out_remove:
-	debugfs_remove_recursive(d->dfs_dir);
-out:
-	err = dent ? PTR_ERR(dent) : -ENODEV;
-	ubi_err(ubi, "cannot create \"%s\" debugfs file or directory, error %d\n",
-		fname, err);
-	return err;
+	d->dfs_chk_io = debugfs_create_file("chk_io", S_IWUSR, d->dfs_dir,
+					    (void *)ubi_num, &dfs_fops);
+
+	d->dfs_chk_fastmap = debugfs_create_file("chk_fastmap", S_IWUSR,
+						 d->dfs_dir, (void *)ubi_num,
+						 &dfs_fops);
+
+	d->dfs_disable_bgt = debugfs_create_file("tst_disable_bgt", S_IWUSR,
+						 d->dfs_dir, (void *)ubi_num,
+						 &dfs_fops);
+
+	d->dfs_emulate_bitflips = debugfs_create_file("tst_emulate_bitflips",
+						      S_IWUSR, d->dfs_dir,
+						      (void *)ubi_num,
+						      &dfs_fops);
+
+	d->dfs_emulate_io_failures = debugfs_create_file("tst_emulate_io_failures",
+							 S_IWUSR, d->dfs_dir,
+							 (void *)ubi_num,
+							 &dfs_fops);
+
+	d->dfs_emulate_power_cut = debugfs_create_file("tst_emulate_power_cut",
+						       S_IWUSR, d->dfs_dir,
+						       (void *)ubi_num,
+						       &dfs_fops);
+	d->dfs_power_cut_min = debugfs_create_file("tst_emulate_power_cut_min",
+						   S_IWUSR, d->dfs_dir,
+						   (void *)ubi_num, &dfs_fops);
+
+	d->dfs_power_cut_max = debugfs_create_file("tst_emulate_power_cut_max",
+						   S_IWUSR, d->dfs_dir,
+						   (void *)ubi_num, &dfs_fops);
+
+	debugfs_create_file("detailed_erase_block_info", S_IRUSR, d->dfs_dir,
+			    (void *)ubi_num, &eraseblk_count_fops);
+
+	return 0;
 }
 
 /**
-- 
2.23.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mtd: no need to check return value of debugfs_create functions
  2019-11-07  8:51 [PATCH] mtd: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-11-07  9:09 ` Miquel Raynal
  2019-11-07  9:15   ` Greg Kroah-Hartman
  2019-11-12 14:19 ` Miquel Raynal
  1 sibling, 1 reply; 5+ messages in thread
From: Miquel Raynal @ 2019-11-07  9:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vignesh Raghavendra, Artem Bityutskiy, Richard Weinberger,
	linux-kernel, Marek Vasut, linux-mtd, Brian Norris

Hi Greg,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote on Thu, 7 Nov
2019 09:51:11 +0100:

> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic should
> never do something different based on this.

I didn't know about this. Is this something new or has it been the rule
since the beginning? In the  case, don't we need a Fixes tag here?

> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: Marek Vasut <marek.vasut@gmail.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Cc: linux-mtd@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[...]

> +
> +	d->dfs_emulate_io_failures = debugfs_create_file("tst_emulate_io_failures",
> +							 S_IWUSR, d->dfs_dir,
> +							 (void *)ubi_num,
> +							 &dfs_fops);
> +
> +	d->dfs_emulate_power_cut = debugfs_create_file("tst_emulate_power_cut",
> +						       S_IWUSR, d->dfs_dir,
> +						       (void *)ubi_num,
> +						       &dfs_fops);

Nitpick: I think we miss an empty line here. I can fix it when applying.

> +	d->dfs_power_cut_min = debugfs_create_file("tst_emulate_power_cut_min",
> +						   S_IWUSR, d->dfs_dir,
> +						   (void *)ubi_num, &dfs_fops);
> +
> +	d->dfs_power_cut_max = debugfs_create_file("tst_emulate_power_cut_max",
> +						   S_IWUSR, d->dfs_dir,
> +						   (void *)ubi_num, &dfs_fops);
> +
> +	debugfs_create_file("detailed_erase_block_info", S_IRUSR, d->dfs_dir,
> +			    (void *)ubi_num, &eraseblk_count_fops);
> +
> +	return 0;
>  }
>  
>  /**

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mtd: no need to check return value of debugfs_create functions
  2019-11-07  9:09 ` Miquel Raynal
@ 2019-11-07  9:15   ` Greg Kroah-Hartman
  2019-11-07 10:09     ` Miquel Raynal
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2019-11-07  9:15 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Vignesh Raghavendra, Artem Bityutskiy, Richard Weinberger,
	linux-kernel, Marek Vasut, linux-mtd, Brian Norris

On Thu, Nov 07, 2019 at 10:09:44AM +0100, Miquel Raynal wrote:
> Hi Greg,
> 
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote on Thu, 7 Nov
> 2019 09:51:11 +0100:
> 
> > When calling debugfs functions, there is no need to ever check the
> > return value.  The function can work or not, but the code logic should
> > never do something different based on this.
> 
> I didn't know about this. Is this something new or has it been the rule
> since the beginning? In the  case, don't we need a Fixes tag here?

It's been the way always, but as of a few kernel releases ago, debugfs
is even more "fault-tolerant" of stuff like this.

And there's no need for a "Fixes:" as this is just work to clean up the
debugfs api and usage (I have a lot more work to do after these types of
changes.)

> 
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Brian Norris <computersforpeace@gmail.com>
> > Cc: Marek Vasut <marek.vasut@gmail.com>
> > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> > Cc: Richard Weinberger <richard@nod.at>
> > Cc: Vignesh Raghavendra <vigneshr@ti.com>
> > Cc: Artem Bityutskiy <dedekind1@gmail.com>
> > Cc: linux-mtd@lists.infradead.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> [...]
> 
> > +
> > +	d->dfs_emulate_io_failures = debugfs_create_file("tst_emulate_io_failures",
> > +							 S_IWUSR, d->dfs_dir,
> > +							 (void *)ubi_num,
> > +							 &dfs_fops);
> > +
> > +	d->dfs_emulate_power_cut = debugfs_create_file("tst_emulate_power_cut",
> > +						       S_IWUSR, d->dfs_dir,
> > +						       (void *)ubi_num,
> > +						       &dfs_fops);
> 
> Nitpick: I think we miss an empty line here. I can fix it when applying.

Ah, oops, sorry about that.

thanks,

greg k-h

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mtd: no need to check return value of debugfs_create functions
  2019-11-07  9:15   ` Greg Kroah-Hartman
@ 2019-11-07 10:09     ` Miquel Raynal
  0 siblings, 0 replies; 5+ messages in thread
From: Miquel Raynal @ 2019-11-07 10:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Vignesh Raghavendra, Artem Bityutskiy, Richard Weinberger,
	linux-kernel, Marek Vasut, linux-mtd, Brian Norris

Hi Greg,

Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote on Thu, 7 Nov
2019 10:15:18 +0100:

> On Thu, Nov 07, 2019 at 10:09:44AM +0100, Miquel Raynal wrote:
> > Hi Greg,
> > 
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote on Thu, 7 Nov
> > 2019 09:51:11 +0100:
> >   
> > > When calling debugfs functions, there is no need to ever check the
> > > return value.  The function can work or not, but the code logic should
> > > never do something different based on this.  
> > 
> > I didn't know about this. Is this something new or has it been the rule
> > since the beginning? In the  case, don't we need a Fixes tag here?  
> 
> It's been the way always, but as of a few kernel releases ago, debugfs
> is even more "fault-tolerant" of stuff like this.
> 
> And there's no need for a "Fixes:" as this is just work to clean up the
> debugfs api and usage (I have a lot more work to do after these types of
> changes.)

Ok, thanks for the clarification.

Cheers!
Miquèl

> 
> >   
> > > Cc: David Woodhouse <dwmw2@infradead.org>
> > > Cc: Brian Norris <computersforpeace@gmail.com>
> > > Cc: Marek Vasut <marek.vasut@gmail.com>
> > > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> > > Cc: Richard Weinberger <richard@nod.at>
> > > Cc: Vignesh Raghavendra <vigneshr@ti.com>
> > > Cc: Artem Bityutskiy <dedekind1@gmail.com>
> > > Cc: linux-mtd@lists.infradead.org
> > > Cc: linux-kernel@vger.kernel.org
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>  
> > 
> > [...]
> >   
> > > +
> > > +	d->dfs_emulate_io_failures = debugfs_create_file("tst_emulate_io_failures",
> > > +							 S_IWUSR, d->dfs_dir,
> > > +							 (void *)ubi_num,
> > > +							 &dfs_fops);
> > > +
> > > +	d->dfs_emulate_power_cut = debugfs_create_file("tst_emulate_power_cut",
> > > +						       S_IWUSR, d->dfs_dir,
> > > +						       (void *)ubi_num,
> > > +						       &dfs_fops);  
> > 
> > Nitpick: I think we miss an empty line here. I can fix it when applying.  
> 
> Ah, oops, sorry about that.
> 
> thanks,
> 
> greg k-h


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mtd: no need to check return value of debugfs_create functions
  2019-11-07  8:51 [PATCH] mtd: no need to check return value of debugfs_create functions Greg Kroah-Hartman
  2019-11-07  9:09 ` Miquel Raynal
@ 2019-11-12 14:19 ` Miquel Raynal
  1 sibling, 0 replies; 5+ messages in thread
From: Miquel Raynal @ 2019-11-12 14:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman, David Woodhouse, Brian Norris, Marek Vasut,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Artem Bityutskiy
  Cc: linux-mtd, linux-kernel

On Thu, 2019-11-07 at 08:51:11 UTC, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic should
> never do something different based on this.
> 
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: Marek Vasut <marek.vasut@gmail.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: Artem Bityutskiy <dedekind1@gmail.com>
> Cc: linux-mtd@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Miquel

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-07  8:51 [PATCH] mtd: no need to check return value of debugfs_create functions Greg Kroah-Hartman
2019-11-07  9:09 ` Miquel Raynal
2019-11-07  9:15   ` Greg Kroah-Hartman
2019-11-07 10:09     ` Miquel Raynal
2019-11-12 14:19 ` Miquel Raynal

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org
	public-inbox-index linux-mtd

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git