public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Stephen Hemminger <shemminger@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [RFC] debugfs: create file error handling
Date: Mon, 18 Jun 2007 13:57:27 -0700	[thread overview]
Message-ID: <20070618205727.GA14857@kroah.com> (raw)
In-Reply-To: <20070618134132.463a6699@localhost.localdomain>

On Mon, Jun 18, 2007 at 01:41:32PM -0700, Stephen Hemminger wrote:
> Shouldn't Debugfs file routines should either return NULL or use ERR_PTR()
> not mixed? The following patch changes the create routines to
> propagate return values.
> 
> --- a/fs/debugfs/file.c	2007-05-30 10:32:32.000000000 -0700
> +++ b/fs/debugfs/file.c	2007-06-18 13:38:01.000000000 -0700
> @@ -83,12 +83,11 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u8, debugfs
>   * 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 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 an error occurs, an invalid pointer will be returned, use
> + * the function IS_ERR() to check. The error code can be extracted
> + * with PTR_ERR().

No, you are forgetting the issue when debugfs is not enabled in the
kernel.

The goal is not to return -ENODEV when it's not configured in, to make
it "simpler" to handle that case.

Check the lkml archives for previous times this has come up and example
code to use to handle the error cases properly.

I agree it isn't the "simplest" way, and if you can suggest something
that is easier, and allow for the code to "easily" determine the option
isn't built in that would be great.

thanks,

greg k-h

  reply	other threads:[~2007-06-18 20:57 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-18 20:41 [RFC] debugfs: create file error handling Stephen Hemminger
2007-06-18 20:57 ` Greg KH [this message]
2007-06-18 21:27   ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070618205727.GA14857@kroah.com \
    --to=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shemminger@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox