Skip to contentSkip to conversation

usb: gadget: gr_udc: debugfs functions return NULL on error

PATCH In review Dan Carpenter 2 participants 3 messages Jan 9, 2014

Subsystems (2)
USB Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 usb: gadget: gr_udc: debugfs functions return NULL on error

Debugfs functions return NULL on error. They return an ERR_PTR if you don't have debugfs configured.

The way it's designed is that normally you are only supposed to test for NULL. In this code, if "dev->dfs_root" is an ERR_PTR then passing it to debugfs_create_file() will not cause a problem because debugfs_create_file() would also just a stub.

Signed-off-by Dan Carpenter <[email protected]>
drivers/usb/gadget/gr_udc.c +2 -2
index 5f9c65959dd2..b34a52171568 100644
@@ -226,13 +226,13 @@ static void gr_dfs_create(struct gr_udc *dev)
 	const char *name = "gr_udc_state";
 
 	dev->dfs_root = debugfs_create_dir(dev_name(dev->dev), NULL);
-	if (IS_ERR(dev->dfs_root)) {
+	if (!dev->dfs_root) {
 		dev_err(dev->dev, "Failed to create debugfs directory\n");
 		return;
 	}
 	dev->dfs_state = debugfs_create_file(name, 0444, dev->dfs_root,
 					     dev, &gr_dfs_fops);
-	if (IS_ERR(dev->dfs_state))
+	if (!dev->dfs_state)
 		dev_err(dev->dev, "Failed to create debugfs file %s\n", name);
 }
maintainer
#1 lore raw
Quoted 19 lines · Dan Carpenter

Debugfs functions return NULL on error. They return an ERR_PTR if you don't have debugfs configured.

The way it's designed is that normally you are only supposed to test for NULL. In this code, if "dev->dfs_root" is an ERR_PTR then passing it to debugfs_create_file() will not cause a problem because debugfs_create_file() would also just a stub.

Signed-off-by Dan Carpenter <[email protected]>
drivers/usb/gadget/gr_udc.c +0 -1
index 5f9c65959dd2..b34a52171568 100644
@@ -226,13 +226,13 @@ static void gr_dfs_create(struct gr_udc *dev)
 	const char *name = "gr_udc_state";
 
 	dev->dfs_root = debugfs_create_dir(dev_name(dev->dev), NULL);
-	if (IS_ERR(dev->dfs_root)) {

drivers/usb/gadget/gr_udc.c static void gr_dfs_create(struct gr_udc *dev) · line 237

Quoted 10 lines
drivers/usb/gadget/gr_udc.c +2 -1
+	if (!dev->dfs_root) {
 		dev_err(dev->dev, "Failed to create debugfs directory\n");
 		return;
 	}
 	dev->dfs_state = debugfs_create_file(name, 0444, dev->dfs_root,
 					     dev, &gr_dfs_fops);
-	if (IS_ERR(dev->dfs_state))
+	if (!dev->dfs_state)
 		dev_err(dev->dev, "Failed to create debugfs file %s\n", name);
 }

Don't even check the return value of the calls, I don't think it matters, right?

greg k-h

author
#2 lore raw
In reply to Dan Carpenter
Quoted 18 lines · Greg Kroah-Hartman
Quoted 17 lines · Dan Carpenter

Debugfs functions return NULL on error. They return an ERR_PTR if you don't have debugfs configured.

The way it's designed is that normally you are only supposed to test for NULL. In this code, if "dev->dfs_root" is an ERR_PTR then passing it to debugfs_create_file() will not cause a problem because debugfs_create_file() would also just a stub.

Signed-off-by Dan Carpenter <[email protected]>
drivers/usb/gadget/gr_udc.c +0 -0
index 5f9c65959dd2..b34a52171568 100644
@@ -226,13 +226,13 @@ static void gr_dfs_create(struct gr_udc *dev)
 	const char *name = "gr_udc_state";
Quoted 16 lines

drivers/usb/gadget/gr_udc.c static void gr_dfs_create(struct gr_udc *dev) · line 237

Quoted 12 lines
drivers/usb/gadget/gr_udc.c +2 -2
 	dev->dfs_root = debugfs_create_dir(dev_name(dev->dev), NULL);
-	if (IS_ERR(dev->dfs_root)) {
+	if (!dev->dfs_root) {
 		dev_err(dev->dev, "Failed to create debugfs directory\n");
 		return;
 	}
 	dev->dfs_state = debugfs_create_file(name, 0444, dev->dfs_root,
 					     dev, &gr_dfs_fops);
-	if (IS_ERR(dev->dfs_state))
+	if (!dev->dfs_state)
 		dev_err(dev->dev, "Failed to create debugfs file %s\n", name);
 }

Don't even check the return value of the calls, I don't think it matters, right?

I assume your question is rhetorical since you wrote debugfs... :P Yes, I looked and if we don't create the initial directory then the files get put in parent directory instead.

I'll resend this.

regards,
dan carpenter