Re: [PATCH V6 1/5] mtd: partitions: add helper for deleting partition

On 21 June 2017 at 00:50, Brian Norris [off-list ref] wrote:

On Fri, May 26, 2017 at 03:14:11PM +0200, Rafał Miłecki wrote:

quoted

From: Rafał Miłecki <redacted>

There are two similar functions handling deletion. One handles single
partition and another whole MTD flash device. They share (duplicate)
some code so it makes sense to add a small helper for that part.

Function del_mtd_partitions has been moved a bit to keep all deleting
stuff together.

Signed-off-by: Rafał Miłecki <redacted>

quoted

+/**
+ * __mtd_del_partition - delete MTD partition
+ *
+ * @priv: internal MTD struct for partition to be deleted
+ *
+ * This function must be called with the partitions mutex locked.
+ */
+static int __mtd_del_partition(struct mtd_part *priv)
+{
+     int err;
+
+     sysfs_remove_files(&priv->mtd.dev.kobj, mtd_partition_attrs);

Notably, the above wasn't called in del_mtd_partitions() previously. Is
that intentional?

As I read the code, we were actually missing this before; either in
error handling, or on device removal. But in either case, the entire
device was going to disappear, so the sysfs files would have been
cleaned up anyway?

Also, is there any chance of double-calling this? I think not, but even
if there is, it looks like the low-level routines are reslient to
non-existent files.

I'm mostly thinking out loud here. I think this is a positive change,
even if it wasn't noted explicitly.

I think that call was missing, however I could make it more clear I'm
adding it. I'll try splitting this change out V7 and add a proper
description.

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html