All of lore.kernel.org
 help / color / mirror / Atom feed
* [01/22] USB: typec: tcpm: no need to check return value of debugfs_create_dir()
@ 2018-05-30 10:30 Heikki Krogerus
  0 siblings, 0 replies; 6+ messages in thread
From: Heikki Krogerus @ 2018-05-30 10:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb, Guenter Roeck

+Guenter

On Tue, May 29, 2018 at 05:30:46PM +0200, 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.
> 
> Clean up the tcpm.c code to not care about this, turns out no one was
> even checking the return value of this function, so it didn't matter.
> 
> Note, I do not think this code can be removed in a running system, as
> the debugfs root directory will stick around, that should be fixed
> someday...
> 
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Revieved-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/tcpm.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 0ccd2ce1eb59..8a201dd53d36 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -566,21 +566,16 @@ DEFINE_SHOW_ATTRIBUTE(tcpm_debug);
>  
>  static struct dentry *rootdir;
>  
> -static int tcpm_debugfs_init(struct tcpm_port *port)
> +static void tcpm_debugfs_init(struct tcpm_port *port)
>  {
>  	mutex_init(&port->logbuffer_lock);
>  	/* /sys/kernel/debug/tcpm/usbcX */
> -	if (!rootdir) {
> +	if (!rootdir)
>  		rootdir = debugfs_create_dir("tcpm", NULL);
> -		if (!rootdir)
> -			return -ENOMEM;
> -	}
>  
>  	port->dentry = debugfs_create_file(dev_name(port->dev),
>  					   S_IFREG | 0444, rootdir,
>  					   port, &tcpm_debug_fops);
> -
> -	return 0;
>  }
>  
>  static void tcpm_debugfs_exit(struct tcpm_port *port)
> @@ -595,7 +590,7 @@ static void tcpm_log(const struct tcpm_port *port, const char *fmt, ...) { }
>  __printf(2, 3)
>  static void tcpm_log_force(struct tcpm_port *port, const char *fmt, ...) { }
>  static void tcpm_log_source_caps(struct tcpm_port *port) { }
> -static int tcpm_debugfs_init(const struct tcpm_port *port) { return 0; }
> +static void tcpm_debugfs_init(const struct tcpm_port *port) { }
>  static void tcpm_debugfs_exit(const struct tcpm_port *port) { }
>  
>  #endif
> -- 
> 2.17.0

Thanks,

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

* [01/22] USB: typec: tcpm: no need to check return value of debugfs_create_dir()
@ 2018-05-31 10:52 Greg Kroah-Hartman
  0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-31 10:52 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Heikki Krogerus, linux-usb

On Wed, May 30, 2018 at 06:42:49AM -0700, Guenter Roeck wrote:
> On 05/30/2018 03:36 AM, Greg Kroah-Hartman wrote:
> > On Wed, May 30, 2018 at 01:30:20PM +0300, Heikki Krogerus wrote:
> > > +Guenter
> > > 
> > > On Tue, May 29, 2018 at 05:30:46PM +0200, 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.
> > > > 
> > > > Clean up the tcpm.c code to not care about this, turns out no one was
> > > > even checking the return value of this function, so it didn't matter.
> > > > 
> > > > Note, I do not think this code can be removed in a running system, as
> > > > the debugfs root directory will stick around, that should be fixed
> > > > someday...
> > 
> > As Guenter pointed out to a different patch in this series, the code is
> > correct as-is, I read it wrong.  I'll delete this sentance from the
> > changelog when I apply it.
> > 
> Well, maybe I did not have enough coffee, but after looking into the code,
> I do think it is missing
> 	debugfs_remove(rootdir);
> in the removal path. See commit c9359f416 in linux-next. You had tried
> to replace that with debugfs_remove_recursive() in the fusb302 driver.

Yes, I think you are correct.

greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [01/22] USB: typec: tcpm: no need to check return value of debugfs_create_dir()
@ 2018-05-30 13:42 Guenter Roeck
  0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2018-05-30 13:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Heikki Krogerus; +Cc: linux-usb

On 05/30/2018 03:36 AM, Greg Kroah-Hartman wrote:
> On Wed, May 30, 2018 at 01:30:20PM +0300, Heikki Krogerus wrote:
>> +Guenter
>>
>> On Tue, May 29, 2018 at 05:30:46PM +0200, 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.
>>>
>>> Clean up the tcpm.c code to not care about this, turns out no one was
>>> even checking the return value of this function, so it didn't matter.
>>>
>>> Note, I do not think this code can be removed in a running system, as
>>> the debugfs root directory will stick around, that should be fixed
>>> someday...
> 
> As Guenter pointed out to a different patch in this series, the code is
> correct as-is, I read it wrong.  I'll delete this sentance from the
> changelog when I apply it.
> 
Well, maybe I did not have enough coffee, but after looking into the code,
I do think it is missing
	debugfs_remove(rootdir);
in the removal path. See commit c9359f416 in linux-next. You had tried
to replace that with debugfs_remove_recursive() in the fusb302 driver.

Thanks,
Guenter
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [01/22] USB: typec: tcpm: no need to check return value of debugfs_create_dir()
@ 2018-05-30 13:35 Guenter Roeck
  0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2018-05-30 13:35 UTC (permalink / raw)
  To: Heikki Krogerus, Greg Kroah-Hartman; +Cc: linux-usb

On 05/30/2018 03:30 AM, Heikki Krogerus wrote:
> +Guenter
> 
> On Tue, May 29, 2018 at 05:30:46PM +0200, 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.
>>
>> Clean up the tcpm.c code to not care about this, turns out no one was
>> even checking the return value of this function, so it didn't matter.
>>
>> Note, I do not think this code can be removed in a running system, as
>> the debugfs root directory will stick around, that should be fixed
>> someday...
>>

Yes. I'll send a patch.

>> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Revieved-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> 
>> ---
>>   drivers/usb/typec/tcpm.c | 11 +++--------
>>   1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
>> index 0ccd2ce1eb59..8a201dd53d36 100644
>> --- a/drivers/usb/typec/tcpm.c
>> +++ b/drivers/usb/typec/tcpm.c
>> @@ -566,21 +566,16 @@ DEFINE_SHOW_ATTRIBUTE(tcpm_debug);
>>   
>>   static struct dentry *rootdir;
>>   
>> -static int tcpm_debugfs_init(struct tcpm_port *port)
>> +static void tcpm_debugfs_init(struct tcpm_port *port)
>>   {
>>   	mutex_init(&port->logbuffer_lock);
>>   	/* /sys/kernel/debug/tcpm/usbcX */
>> -	if (!rootdir) {
>> +	if (!rootdir)
>>   		rootdir = debugfs_create_dir("tcpm", NULL);
>> -		if (!rootdir)
>> -			return -ENOMEM;
>> -	}
>>   
>>   	port->dentry = debugfs_create_file(dev_name(port->dev),
>>   					   S_IFREG | 0444, rootdir,
>>   					   port, &tcpm_debug_fops);
>> -
>> -	return 0;
>>   }
>>   
>>   static void tcpm_debugfs_exit(struct tcpm_port *port)
>> @@ -595,7 +590,7 @@ static void tcpm_log(const struct tcpm_port *port, const char *fmt, ...) { }
>>   __printf(2, 3)
>>   static void tcpm_log_force(struct tcpm_port *port, const char *fmt, ...) { }
>>   static void tcpm_log_source_caps(struct tcpm_port *port) { }
>> -static int tcpm_debugfs_init(const struct tcpm_port *port) { return 0; }
>> +static void tcpm_debugfs_init(const struct tcpm_port *port) { }
>>   static void tcpm_debugfs_exit(const struct tcpm_port *port) { }
>>   
>>   #endif
>> -- 
>> 2.17.0
> 
> Thanks,
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [01/22] USB: typec: tcpm: no need to check return value of debugfs_create_dir()
@ 2018-05-30 10:36 Greg Kroah-Hartman
  0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-30 10:36 UTC (permalink / raw)
  To: Heikki Krogerus; +Cc: linux-usb, Guenter Roeck

On Wed, May 30, 2018 at 01:30:20PM +0300, Heikki Krogerus wrote:
> +Guenter
> 
> On Tue, May 29, 2018 at 05:30:46PM +0200, 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.
> > 
> > Clean up the tcpm.c code to not care about this, turns out no one was
> > even checking the return value of this function, so it didn't matter.
> > 
> > Note, I do not think this code can be removed in a running system, as
> > the debugfs root directory will stick around, that should be fixed
> > someday...

As Guenter pointed out to a different patch in this series, the code is
correct as-is, I read it wrong.  I'll delete this sentance from the
changelog when I apply it.

thanks,

greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [01/22] USB: typec: tcpm: no need to check return value of debugfs_create_dir()
@ 2018-05-29 15:30 Greg Kroah-Hartman
  0 siblings, 0 replies; 6+ messages in thread
From: Greg Kroah-Hartman @ 2018-05-29 15:30 UTC (permalink / raw)
  To: linux-usb; +Cc: Greg Kroah-Hartman, Heikki Krogerus

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.

Clean up the tcpm.c code to not care about this, turns out no one was
even checking the return value of this function, so it didn't matter.

Note, I do not think this code can be removed in a running system, as
the debugfs root directory will stick around, that should be fixed
someday...

Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/usb/typec/tcpm.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 0ccd2ce1eb59..8a201dd53d36 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -566,21 +566,16 @@ DEFINE_SHOW_ATTRIBUTE(tcpm_debug);
 
 static struct dentry *rootdir;
 
-static int tcpm_debugfs_init(struct tcpm_port *port)
+static void tcpm_debugfs_init(struct tcpm_port *port)
 {
 	mutex_init(&port->logbuffer_lock);
 	/* /sys/kernel/debug/tcpm/usbcX */
-	if (!rootdir) {
+	if (!rootdir)
 		rootdir = debugfs_create_dir("tcpm", NULL);
-		if (!rootdir)
-			return -ENOMEM;
-	}
 
 	port->dentry = debugfs_create_file(dev_name(port->dev),
 					   S_IFREG | 0444, rootdir,
 					   port, &tcpm_debug_fops);
-
-	return 0;
 }
 
 static void tcpm_debugfs_exit(struct tcpm_port *port)
@@ -595,7 +590,7 @@ static void tcpm_log(const struct tcpm_port *port, const char *fmt, ...) { }
 __printf(2, 3)
 static void tcpm_log_force(struct tcpm_port *port, const char *fmt, ...) { }
 static void tcpm_log_source_caps(struct tcpm_port *port) { }
-static int tcpm_debugfs_init(const struct tcpm_port *port) { return 0; }
+static void tcpm_debugfs_init(const struct tcpm_port *port) { }
 static void tcpm_debugfs_exit(const struct tcpm_port *port) { }
 
 #endif

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

end of thread, other threads:[~2018-05-31 10:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-30 10:30 [01/22] USB: typec: tcpm: no need to check return value of debugfs_create_dir() Heikki Krogerus
  -- strict thread matches above, loose matches on Subject: below --
2018-05-31 10:52 Greg Kroah-Hartman
2018-05-30 13:42 Guenter Roeck
2018-05-30 13:35 Guenter Roeck
2018-05-30 10:36 Greg Kroah-Hartman
2018-05-29 15:30 Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.