Re: [PATCH] fs: configfs: don't return anything from drop_link

[Christoph Hellwig]

Thanks a lot Andrzej!

I've applied the patch, but I undid the reformatting of the nvmet
code to keep the patch as simple as possible.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] fs: configfs: don't return anything from drop_link

[Andrzej Pietrasiewicz]

Documentation/filesystems/configfs/configfs.txt says:

"When unlink(2) is called on the symbolic link, the source item is
notified via the ->drop_link() method.  Like the ->drop_item() method,
this is a void function and cannot return failure."

The ->drop_item() is indeed a void function, the ->drop_link() is
actually not. This, together with the fact that the value of ->drop_link()
is silently ignored suggests, that it is the ->drop_link() return
type that should be corrected and changed to void.

This patch changes drop_link() signature and all its users.

Compile-tested only! It needs Tested-by from respective subsystems.

Signed-off-by: Andrzej Pietrasiewicz 
---
 Documentation/filesystems/configfs/configfs.txt |  2 +-
 drivers/nvme/target/configfs.c  | 46 ++---
 drivers/target/target_core_fabric_configfs.c|  7 ++--
 drivers/usb/gadget/configfs.c   |  8 ++---
 drivers/usb/gadget/function/uvc_configfs.c  | 25 +++---
 include/linux/configfs.h|  2 +-
 6 files changed, 31 insertions(+), 59 deletions(-)

diff --git a/Documentation/filesystems/configfs/configfs.txt 
b/Documentation/filesystems/configfs/configfs.txt
index 8ec9136..3828e85 100644
--- a/Documentation/filesystems/configfs/configfs.txt
+++ b/Documentation/filesystems/configfs/configfs.txt
@@ -174,7 +174,7 @@ among other things.  For that, it needs a type.
void (*release)(struct config_item *);
int (*allow_link)(struct config_item *src,
  struct config_item *target);
-   int (*drop_link)(struct config_item *src,
+   void (*drop_link)(struct config_item *src,
 struct config_item *target);
};
 
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index af5e2dc..9ee1490 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -466,7 +466,7 @@ static int nvmet_port_subsys_allow_link(struct config_item 
*parent,
return ret;
 }
 
-static int nvmet_port_subsys_drop_link(struct config_item *parent,
+static void nvmet_port_subsys_drop_link(struct config_item *parent,
struct config_item *target)
 {
struct nvmet_port *port = to_nvmet_port(parent->ci_parent);
@@ -474,21 +474,16 @@ static int nvmet_port_subsys_drop_link(struct config_item 
*parent,
struct nvmet_subsys_link *p;
 
down_write(_config_sem);
-   list_for_each_entry(p, >subsystems, entry) {
-   if (p->subsys == subsys)
-   goto found;
-   }
-   up_write(_config_sem);
-   return -EINVAL;
-
-found:
-   list_del(>entry);
-   nvmet_genctr++;
-   if (list_empty(>subsystems))
-   nvmet_disable_port(port);
+   list_for_each_entry(p, >subsystems, entry)
+   if (p->subsys == subsys) {
+   list_del(>entry);
+   nvmet_genctr++;
+   if (list_empty(>subsystems))
+   nvmet_disable_port(port);
+   kfree(p);
+   break;
+   }
up_write(_config_sem);
-   kfree(p);
-   return 0;
 }
 
 static struct configfs_item_operations nvmet_port_subsys_item_ops = {
@@ -542,7 +537,7 @@ static int nvmet_allowed_hosts_allow_link(struct 
config_item *parent,
return ret;
 }
 
-static int nvmet_allowed_hosts_drop_link(struct config_item *parent,
+static void nvmet_allowed_hosts_drop_link(struct config_item *parent,
struct config_item *target)
 {
struct nvmet_subsys *subsys = to_subsys(parent->ci_parent);
@@ -550,19 +545,14 @@ static int nvmet_allowed_hosts_drop_link(struct 
config_item *parent,
struct nvmet_host_link *p;
 
down_write(_config_sem);
-   list_for_each_entry(p, >hosts, entry) {
-   if (!strcmp(nvmet_host_name(p->host), nvmet_host_name(host)))
-   goto found;
-   }
-   up_write(_config_sem);
-   return -EINVAL;
-
-found:
-   list_del(>entry);
-   nvmet_genctr++;
+   list_for_each_entry(p, >hosts, entry)
+   if (!strcmp(nvmet_host_name(p->host), nvmet_host_name(host))) {
+   list_del(>entry);
+   nvmet_genctr++;
+   kfree(p);
+   break;
+   }
up_write(_config_sem);
-   kfree(p);
-   return 0;
 }
 
 static struct configfs_item_operations nvmet_allowed_hosts_item_ops = {
diff --git a/drivers/target/target_core_fabric_configfs.c 
b/drivers/target/target_core_fabric_configfs.c
index 31a096a..d8a16ca 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -137,7 +137,7 @@ static int target_fabric_mappedlun_link(
return core_dev_add_initiator_node_lun_acl(se_tpg, lacl,