linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] debugfs helpers cleanups
@ 2019-04-15  8:25 Ronald Tschalär
  2019-04-15  8:25 ` [PATCH 1/2] debugfs: update documented return values of debugfs helpers Ronald Tschalär
  2019-04-15  8:25 ` [PATCH 2/2] debugfs: make return value of all debugfs helpers consistent Ronald Tschalär
  0 siblings, 2 replies; 7+ messages in thread
From: Ronald Tschalär @ 2019-04-15  8:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jonathan Corbet, Rafael J. Wysocki
  Cc: Andy Shevchenko, linux-doc, linux-kernel

commit ff9fb72bc077 ("debugfs: return error values, not NULL") changed
the return values of various debugfs functions in fs/debugfs/inode.c but
did not update the documentation or behaviour of affected functions in
fs/debugfs/file.c . These patches attempt to remedy this.

Ronald Tschalär (2):
  debugfs: update documented return values of debugfs helpers
  debugfs: make return value of all debugfs helpers consistent

 Documentation/filesystems/debugfs.txt | 16 +++---
 fs/debugfs/file.c                     | 79 ++++++++++++---------------
 2 files changed, 45 insertions(+), 50 deletions(-)

-- 
2.20.1


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

* [PATCH 1/2] debugfs: update documented return values of debugfs helpers
  2019-04-15  8:25 [PATCH 0/2] debugfs helpers cleanups Ronald Tschalär
@ 2019-04-15  8:25 ` Ronald Tschalär
  2019-04-15  8:25 ` [PATCH 2/2] debugfs: make return value of all debugfs helpers consistent Ronald Tschalär
  1 sibling, 0 replies; 7+ messages in thread
From: Ronald Tschalär @ 2019-04-15  8:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jonathan Corbet, Rafael J. Wysocki
  Cc: Andy Shevchenko, linux-doc, linux-kernel

Since commit ff9fb72bc077 ("debugfs: return error values, not NULL")
these helper functions do not return NULL anymore (with the exception
of debugfs_create_u32_array()).

Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
---
 Documentation/filesystems/debugfs.txt | 16 +++---
 fs/debugfs/file.c                     | 77 ++++++++++++---------------
 2 files changed, 44 insertions(+), 49 deletions(-)

diff --git a/Documentation/filesystems/debugfs.txt b/Documentation/filesystems/debugfs.txt
index 4f45f71149cb..4a0a9c3f4af6 100644
--- a/Documentation/filesystems/debugfs.txt
+++ b/Documentation/filesystems/debugfs.txt
@@ -31,10 +31,10 @@ This call, if successful, will make a directory called name underneath the
 indicated parent directory.  If parent is NULL, the directory will be
 created in the debugfs root.  On success, the return value is a struct
 dentry pointer which can be used to create files in the directory (and to
-clean it up at the end).  A NULL return value indicates that something went
-wrong.  If ERR_PTR(-ENODEV) is returned, that is an indication that the
-kernel has been built without debugfs support and none of the functions
-described below will work.
+clean it up at the end).  An ERR_PTR(-ERROR) return value indicates that
+something went wrong.  If ERR_PTR(-ENODEV) is returned, that is an
+indication that the kernel has been built without debugfs support and none
+of the functions described below will work.
 
 The most general way to create a file within a debugfs directory is with:
 
@@ -48,8 +48,9 @@ should hold the file, data will be stored in the i_private field of the
 resulting inode structure, and fops is a set of file operations which
 implement the file's behavior.  At a minimum, the read() and/or write()
 operations should be provided; others can be included as needed.  Again,
-the return value will be a dentry pointer to the created file, NULL for
-error, or ERR_PTR(-ENODEV) if debugfs support is missing.
+the return value will be a dentry pointer to the created file,
+ERR_PTR(-ERROR) on error, or ERR_PTR(-ENODEV) if debugfs support is
+missing.
 
 Create a file with an initial size, the following function can be used
 instead:
@@ -214,7 +215,8 @@ can be removed with:
 
     void debugfs_remove(struct dentry *dentry);
 
-The dentry value can be NULL, in which case nothing will be removed.
+The dentry value can be NULL or an error value, in which case nothing will
+be removed.
 
 Once upon a time, debugfs users were required to remember the dentry
 pointer for every debugfs file they created so that all files could be
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 4fce1da7db23..ddd708b09fa1 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -394,12 +394,11 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_u8_wo, NULL, debugfs_u8_set, "%llu\n");
  * This function will return a pointer to a dentry if it succeeds.  This
  * pointer must be passed to the debugfs_remove() function when the file is
  * to be removed (no automatic cleanup happens if your module is unloaded,
- * you are responsible here.)  If an error occurs, %NULL will be returned.
+ * you are responsible here.)  If an error occurs, %ERR_PTR(-ERROR) will be
+ * returned.
  *
- * If debugfs is not enabled in the kernel, the value -%ENODEV will be
- * returned.  It is not wise to check for this value, but rather, check for
- * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
- * code.
+ * If debugfs is not enabled in the kernel, the value %ERR_PTR(-ENODEV) will
+ * be returned.
  */
 struct dentry *debugfs_create_u8(const char *name, umode_t mode,
 				 struct dentry *parent, u8 *value)
@@ -440,12 +439,11 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_u16_wo, NULL, debugfs_u16_set, "%llu\n");
  * This function will return a pointer to a dentry if it succeeds.  This
  * pointer must be passed to the debugfs_remove() function when the file is
  * to be removed (no automatic cleanup happens if your module is unloaded,
- * you are responsible here.)  If an error occurs, %NULL will be returned.
+ * you are responsible here.)  If an error occurs, %ERR_PTR(-ERROR) will be
+ * returned.
  *
- * If debugfs is not enabled in the kernel, the value -%ENODEV will be
- * returned.  It is not wise to check for this value, but rather, check for
- * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
- * code.
+ * If debugfs is not enabled in the kernel, the value %ERR_PTR(-ENODEV) will
+ * be returned.
  */
 struct dentry *debugfs_create_u16(const char *name, umode_t mode,
 				  struct dentry *parent, u16 *value)
@@ -486,12 +484,11 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_u32_wo, NULL, debugfs_u32_set, "%llu\n");
  * This function will return a pointer to a dentry if it succeeds.  This
  * pointer must be passed to the debugfs_remove() function when the file is
  * to be removed (no automatic cleanup happens if your module is unloaded,
- * you are responsible here.)  If an error occurs, %NULL will be returned.
+ * you are responsible here.)  If an error occurs, %ERR_PTR(-ERROR) will be
+ * returned.
  *
- * If debugfs is not enabled in the kernel, the value -%ENODEV will be
- * returned.  It is not wise to check for this value, but rather, check for
- * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
- * code.
+ * If debugfs is not enabled in the kernel, the value %ERR_PTR(-ENODEV) will
+ * be returned.
  */
 struct dentry *debugfs_create_u32(const char *name, umode_t mode,
 				 struct dentry *parent, u32 *value)
@@ -533,12 +530,11 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_u64_wo, NULL, debugfs_u64_set, "%llu\n");
  * This function will return a pointer to a dentry if it succeeds.  This
  * pointer must be passed to the debugfs_remove() function when the file is
  * to be removed (no automatic cleanup happens if your module is unloaded,
- * you are responsible here.)  If an error occurs, %NULL will be returned.
+ * you are responsible here.)  If an error occurs, %ERR_PTR(-ERROR) will be
+ * returned.
  *
- * If debugfs is not enabled in the kernel, the value -%ENODEV will be
- * returned.  It is not wise to check for this value, but rather, check for
- * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
- * code.
+ * If debugfs is not enabled in the kernel, the value %ERR_PTR(-ENODEV) will
+ * be returned.
  */
 struct dentry *debugfs_create_u64(const char *name, umode_t mode,
 				 struct dentry *parent, u64 *value)
@@ -582,12 +578,11 @@ DEFINE_DEBUGFS_ATTRIBUTE(fops_ulong_wo, NULL, debugfs_ulong_set, "%llu\n");
  * This function will return a pointer to a dentry if it succeeds.  This
  * pointer must be passed to the debugfs_remove() function when the file is
  * to be removed (no automatic cleanup happens if your module is unloaded,
- * you are responsible here.)  If an error occurs, %NULL will be returned.
+ * you are responsible here.)  If an error occurs, %ERR_PTR(-ERROR) will be
+ * returned.
  *
- * If debugfs is not enabled in the kernel, the value -%ENODEV will be
- * returned.  It is not wise to check for this value, but rather, check for
- * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
- * code.
+ * If debugfs is not enabled in the kernel, the value %ERR_PTR(-ENODEV) will
+ * be returned.
  */
 struct dentry *debugfs_create_ulong(const char *name, umode_t mode,
 				    struct dentry *parent, unsigned long *value)
@@ -850,12 +845,11 @@ static const struct file_operations fops_bool_wo = {
  * This function will return a pointer to a dentry if it succeeds.  This
  * pointer must be passed to the debugfs_remove() function when the file is
  * to be removed (no automatic cleanup happens if your module is unloaded,
- * you are responsible here.)  If an error occurs, %NULL will be returned.
+ * you are responsible here.)  If an error occurs, %ERR_PTR(-ERROR) will be
+ * returned.
  *
- * If debugfs is not enabled in the kernel, the value -%ENODEV will be
- * returned.  It is not wise to check for this value, but rather, check for
- * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
- * code.
+ * If debugfs is not enabled in the kernel, the value %ERR_PTR(-ENODEV) will
+ * be returned.
  */
 struct dentry *debugfs_create_bool(const char *name, umode_t mode,
 				   struct dentry *parent, bool *value)
@@ -904,12 +898,11 @@ static const struct file_operations fops_blob = {
  * This function will return a pointer to a dentry if it succeeds.  This
  * pointer must be passed to the debugfs_remove() function when the file is
  * to be removed (no automatic cleanup happens if your module is unloaded,
- * you are responsible here.)  If an error occurs, %NULL will be returned.
+ * you are responsible here.)  If an error occurs, %ERR_PTR(-ERROR) will be
+ * returned.
  *
- * If debugfs is not enabled in the kernel, the value -%ENODEV will be
- * returned.  It is not wise to check for this value, but rather, check for
- * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
- * code.
+ * If debugfs is not enabled in the kernel, the value %ERR_PTR(-ENODEV) will
+ * be returned.
  */
 struct dentry *debugfs_create_blob(const char *name, umode_t mode,
 				   struct dentry *parent,
@@ -1005,8 +998,9 @@ static const struct file_operations u32_array_fops = {
  * Writing is not supported. Seek within the file is also not supported.
  * Once array is created its size can not be changed.
  *
- * The function returns a pointer to dentry on success. If debugfs is not
- * enabled in the kernel, the value -%ENODEV will be returned.
+ * The function returns a pointer to dentry on success. If an error occurs,
+ * %ERR_PTR(-ERROR) or NULL will be returned. If debugfs is not enabled in
+ * the kernel, the value %ERR_PTR(-ENODEV) will be returned.
  */
 struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
 					    struct dentry *parent,
@@ -1102,12 +1096,11 @@ static const struct file_operations fops_regset32 = {
  * This function will return a pointer to a dentry if it succeeds.  This
  * pointer must be passed to the debugfs_remove() function when the file is
  * to be removed (no automatic cleanup happens if your module is unloaded,
- * you are responsible here.)  If an error occurs, %NULL will be returned.
+ * you are responsible here.)  If an error occurs, %ERR_PTR(-ERROR) will be
+ * returned.
  *
- * If debugfs is not enabled in the kernel, the value -%ENODEV will be
- * returned.  It is not wise to check for this value, but rather, check for
- * %NULL or !%NULL instead as to eliminate the need for #ifdef in the calling
- * code.
+ * If debugfs is not enabled in the kernel, the value %ERR_PTR(-ENODEV) will
+ * be returned.
  */
 struct dentry *debugfs_create_regset32(const char *name, umode_t mode,
 				       struct dentry *parent,
-- 
2.20.1


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

* [PATCH 2/2] debugfs: make return value of all debugfs helpers consistent
  2019-04-15  8:25 [PATCH 0/2] debugfs helpers cleanups Ronald Tschalär
  2019-04-15  8:25 ` [PATCH 1/2] debugfs: update documented return values of debugfs helpers Ronald Tschalär
@ 2019-04-15  8:25 ` Ronald Tschalär
  2019-04-15  8:48   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 7+ messages in thread
From: Ronald Tschalär @ 2019-04-15  8:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jonathan Corbet, Rafael J. Wysocki
  Cc: Andy Shevchenko, linux-doc, linux-kernel

Since commit ff9fb72bc077 ("debugfs: return error values, not NULL")
almost all the debugfs helpers have stopped returning NULL. The lone
holdeout was debugfs_create_u32_array(). So fix that.

Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
---
 fs/debugfs/file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index ddd708b09fa1..bb706d073782 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -999,8 +999,8 @@ static const struct file_operations u32_array_fops = {
  * Once array is created its size can not be changed.
  *
  * The function returns a pointer to dentry on success. If an error occurs,
- * %ERR_PTR(-ERROR) or NULL will be returned. If debugfs is not enabled in
- * the kernel, the value %ERR_PTR(-ENODEV) will be returned.
+ * %ERR_PTR(-ERROR) will be returned. If debugfs is not enabled in the kernel,
+ * the value %ERR_PTR(-ENODEV) will be returned.
  */
 struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
 					    struct dentry *parent,
@@ -1009,7 +1009,7 @@ struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
 	struct array_data *data = kmalloc(sizeof(*data), GFP_KERNEL);
 
 	if (data == NULL)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	data->array = array;
 	data->elements = elements;
-- 
2.20.1


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

* Re: [PATCH 2/2] debugfs: make return value of all debugfs helpers consistent
  2019-04-15  8:25 ` [PATCH 2/2] debugfs: make return value of all debugfs helpers consistent Ronald Tschalär
@ 2019-04-15  8:48   ` Greg Kroah-Hartman
  2019-04-15 23:29     ` Life is hard, and then you die
  0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-15  8:48 UTC (permalink / raw)
  To: Ronald Tschalär
  Cc: Jonathan Corbet, Rafael J. Wysocki, Andy Shevchenko, linux-doc,
	linux-kernel

On Mon, Apr 15, 2019 at 01:25:06AM -0700, Ronald Tschalär wrote:
> Since commit ff9fb72bc077 ("debugfs: return error values, not NULL")
> almost all the debugfs helpers have stopped returning NULL. The lone
> holdeout was debugfs_create_u32_array(). So fix that.
> 
> Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
> ---
>  fs/debugfs/file.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index ddd708b09fa1..bb706d073782 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -999,8 +999,8 @@ static const struct file_operations u32_array_fops = {
>   * Once array is created its size can not be changed.
>   *
>   * The function returns a pointer to dentry on success. If an error occurs,
> - * %ERR_PTR(-ERROR) or NULL will be returned. If debugfs is not enabled in
> - * the kernel, the value %ERR_PTR(-ENODEV) will be returned.
> + * %ERR_PTR(-ERROR) will be returned. If debugfs is not enabled in the kernel,
> + * the value %ERR_PTR(-ENODEV) will be returned.
>   */
>  struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
>  					    struct dentry *parent,
> @@ -1009,7 +1009,7 @@ struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
>  	struct array_data *data = kmalloc(sizeof(*data), GFP_KERNEL);
>  
>  	if (data == NULL)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
>  
>  	data->array = array;
>  	data->elements = elements;

There is only one caller of this function in the kernel now, and it does
not even care about the return value at all, so we should just remove
the return value entirely as that's the easiest and best thing to do
here.  I was going to start doing this slowly over time, but as you are
touching the function now, might as well do it here :)

thanks,

greg k-h

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

* Re: [PATCH 2/2] debugfs: make return value of all debugfs helpers consistent
  2019-04-15  8:48   ` Greg Kroah-Hartman
@ 2019-04-15 23:29     ` Life is hard, and then you die
  2019-04-16 13:29       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: Life is hard, and then you die @ 2019-04-15 23:29 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jonathan Corbet, Rafael J. Wysocki, Andy Shevchenko, linux-doc,
	linux-kernel


  Hi Greg,

On Mon, Apr 15, 2019 at 10:48:44AM +0200, Greg Kroah-Hartman wrote:
> On Mon, Apr 15, 2019 at 01:25:06AM -0700, Ronald Tschalär wrote:
> > Since commit ff9fb72bc077 ("debugfs: return error values, not NULL")
> > almost all the debugfs helpers have stopped returning NULL. The lone
> > holdeout was debugfs_create_u32_array(). So fix that.
> > 
> > Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
> > ---
> >  fs/debugfs/file.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> > index ddd708b09fa1..bb706d073782 100644
> > --- a/fs/debugfs/file.c
> > +++ b/fs/debugfs/file.c
> > @@ -999,8 +999,8 @@ static const struct file_operations u32_array_fops = {
> >   * Once array is created its size can not be changed.
> >   *
> >   * The function returns a pointer to dentry on success. If an error occurs,
> > - * %ERR_PTR(-ERROR) or NULL will be returned. If debugfs is not enabled in
> > - * the kernel, the value %ERR_PTR(-ENODEV) will be returned.
> > + * %ERR_PTR(-ERROR) will be returned. If debugfs is not enabled in the kernel,
> > + * the value %ERR_PTR(-ENODEV) will be returned.
> >   */
> >  struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
> >  					    struct dentry *parent,
> > @@ -1009,7 +1009,7 @@ struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
> >  	struct array_data *data = kmalloc(sizeof(*data), GFP_KERNEL);
> >  
> >  	if (data == NULL)
> > -		return NULL;
> > +		return ERR_PTR(-ENOMEM);
> >  
> >  	data->array = array;
> >  	data->elements = elements;
> 
> There is only one caller of this function in the kernel now, and it does
> not even care about the return value at all, so we should just remove
> the return value entirely as that's the easiest and best thing to do
> here.

Interesting argument: since this is a helper/library function, and
therefore potentially used in the future by others, it seems to me
that consistency with the other functions and providing error feedback
would be important.

> I was going to start doing this slowly over time, but as you are
> touching the function now, might as well do it here :)

Are you saying the plan is to make all these helpers return void?


  Cheers,

  Ronald


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

* Re: [PATCH 2/2] debugfs: make return value of all debugfs helpers consistent
  2019-04-15 23:29     ` Life is hard, and then you die
@ 2019-04-16 13:29       ` Greg Kroah-Hartman
  2019-04-16 13:46         ` [PATCH] debugfs: make debugfs_create_u32_array() return void Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-16 13:29 UTC (permalink / raw)
  To: Life is hard, and then you die
  Cc: Jonathan Corbet, Rafael J. Wysocki, Andy Shevchenko, linux-doc,
	linux-kernel

On Mon, Apr 15, 2019 at 04:29:59PM -0700, Life is hard, and then you die wrote:
> 
>   Hi Greg,
> 
> On Mon, Apr 15, 2019 at 10:48:44AM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Apr 15, 2019 at 01:25:06AM -0700, Ronald Tschalär wrote:
> > > Since commit ff9fb72bc077 ("debugfs: return error values, not NULL")
> > > almost all the debugfs helpers have stopped returning NULL. The lone
> > > holdeout was debugfs_create_u32_array(). So fix that.
> > > 
> > > Signed-off-by: Ronald Tschalär <ronald@innovation.ch>
> > > ---
> > >  fs/debugfs/file.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> > > index ddd708b09fa1..bb706d073782 100644
> > > --- a/fs/debugfs/file.c
> > > +++ b/fs/debugfs/file.c
> > > @@ -999,8 +999,8 @@ static const struct file_operations u32_array_fops = {
> > >   * Once array is created its size can not be changed.
> > >   *
> > >   * The function returns a pointer to dentry on success. If an error occurs,
> > > - * %ERR_PTR(-ERROR) or NULL will be returned. If debugfs is not enabled in
> > > - * the kernel, the value %ERR_PTR(-ENODEV) will be returned.
> > > + * %ERR_PTR(-ERROR) will be returned. If debugfs is not enabled in the kernel,
> > > + * the value %ERR_PTR(-ENODEV) will be returned.
> > >   */
> > >  struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
> > >  					    struct dentry *parent,
> > > @@ -1009,7 +1009,7 @@ struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
> > >  	struct array_data *data = kmalloc(sizeof(*data), GFP_KERNEL);
> > >  
> > >  	if (data == NULL)
> > > -		return NULL;
> > > +		return ERR_PTR(-ENOMEM);
> > >  
> > >  	data->array = array;
> > >  	data->elements = elements;
> > 
> > There is only one caller of this function in the kernel now, and it does
> > not even care about the return value at all, so we should just remove
> > the return value entirely as that's the easiest and best thing to do
> > here.
> 
> Interesting argument: since this is a helper/library function, and
> therefore potentially used in the future by others, it seems to me
> that consistency with the other functions and providing error feedback
> would be important.

Not if you never actually use the return value for anything :)

> > I was going to start doing this slowly over time, but as you are
> > touching the function now, might as well do it here :)
> 
> Are you saying the plan is to make all these helpers return void?

Yes, no caller should do anything "different" based on a return value of
a debugfs call.  Note, sometimes you do want to save off dentries for
some files that you later remove, but that's it.

thanks,

greg k-h

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

* [PATCH] debugfs: make debugfs_create_u32_array() return void
  2019-04-16 13:29       ` Greg Kroah-Hartman
@ 2019-04-16 13:46         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2019-04-16 13:46 UTC (permalink / raw)
  To: Life is hard, and then you die
  Cc: Jonathan Corbet, Rafael J. Wysocki, Andy Shevchenko, linux-doc,
	linux-kernel

The single user of debugfs_create_u32_array() does not care about the
return value of it, so make it return void as there is no need to do
anything with the return value.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 Documentation/filesystems/debugfs.txt |  2 +-
 fs/debugfs/file.c                     | 13 ++++---------
 include/linux/debugfs.h               | 12 +++++-------
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/Documentation/filesystems/debugfs.txt b/Documentation/filesystems/debugfs.txt
index 4f45f71149cb..6320e86d2294 100644
--- a/Documentation/filesystems/debugfs.txt
+++ b/Documentation/filesystems/debugfs.txt
@@ -168,7 +168,7 @@ byte offsets over a base for the register block.
 
 If you want to dump an u32 array in debugfs, you can create file with:
 
-    struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
+    void debugfs_create_u32_array(const char *name, umode_t mode,
 			struct dentry *parent,
 			u32 *array, u32 elements);
 
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 4fce1da7db23..2c17039c6287 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -1004,24 +1004,19 @@ static const struct file_operations u32_array_fops = {
  * @array as data. If the @mode variable is so set it can be read from.
  * Writing is not supported. Seek within the file is also not supported.
  * Once array is created its size can not be changed.
- *
- * The function returns a pointer to dentry on success. If debugfs is not
- * enabled in the kernel, the value -%ENODEV will be returned.
  */
-struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
-					    struct dentry *parent,
-					    u32 *array, u32 elements)
+void debugfs_create_u32_array(const char *name, umode_t mode,
+			      struct dentry *parent, u32 *array, u32 elements)
 {
 	struct array_data *data = kmalloc(sizeof(*data), GFP_KERNEL);
 
 	if (data == NULL)
-		return NULL;
+		return;
 
 	data->array = array;
 	data->elements = elements;
 
-	return debugfs_create_file_unsafe(name, mode, parent, data,
-					&u32_array_fops);
+	debugfs_create_file_unsafe(name, mode, parent, data, &u32_array_fops);
 }
 EXPORT_SYMBOL_GPL(debugfs_create_u32_array);
 
diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h
index 3b0ba54cc4d5..58424eb3b329 100644
--- a/include/linux/debugfs.h
+++ b/include/linux/debugfs.h
@@ -133,9 +133,8 @@ struct dentry *debugfs_create_regset32(const char *name, umode_t mode,
 void debugfs_print_regs32(struct seq_file *s, const struct debugfs_reg32 *regs,
 			  int nregs, void __iomem *base, char *prefix);
 
-struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
-					struct dentry *parent,
-					u32 *array, u32 elements);
+void debugfs_create_u32_array(const char *name, umode_t mode,
+			      struct dentry *parent, u32 *array, u32 elements);
 
 struct dentry *debugfs_create_devm_seqfile(struct device *dev, const char *name,
 					   struct dentry *parent,
@@ -353,11 +352,10 @@ static inline bool debugfs_initialized(void)
 	return false;
 }
 
-static inline struct dentry *debugfs_create_u32_array(const char *name, umode_t mode,
-					struct dentry *parent,
-					u32 *array, u32 elements)
+static inline void debugfs_create_u32_array(const char *name, umode_t mode,
+					    struct dentry *parent, u32 *array,
+					    u32 elements)
 {
-	return ERR_PTR(-ENODEV);
 }
 
 static inline struct dentry *debugfs_create_devm_seqfile(struct device *dev,
-- 
2.21.0


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

end of thread, other threads:[~2019-04-16 14:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-15  8:25 [PATCH 0/2] debugfs helpers cleanups Ronald Tschalär
2019-04-15  8:25 ` [PATCH 1/2] debugfs: update documented return values of debugfs helpers Ronald Tschalär
2019-04-15  8:25 ` [PATCH 2/2] debugfs: make return value of all debugfs helpers consistent Ronald Tschalär
2019-04-15  8:48   ` Greg Kroah-Hartman
2019-04-15 23:29     ` Life is hard, and then you die
2019-04-16 13:29       ` Greg Kroah-Hartman
2019-04-16 13:46         ` [PATCH] debugfs: make debugfs_create_u32_array() return void Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).