From: Stephen Hemminger <shemminger@linux-foundation.org>
To: Greg KH <greg@kroah.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [RFC] debugfs: create file error handling
Date: Mon, 18 Jun 2007 14:27:13 -0700 [thread overview]
Message-ID: <20070618142713.4d030a42@localhost.localdomain> (raw)
In-Reply-To: <20070618205727.GA14857@kroah.com>
On Mon, 18 Jun 2007 13:57:27 -0700
Greg KH <greg@kroah.com> wrote:
> 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.
But it makes sense to pass error values back not just: create failed
sorry!
>
> 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.
Checking for ENODEV works fine in either case.
--
Stephen Hemminger <shemminger@linux-foundation.org>
prev parent reply other threads:[~2007-06-18 21:27 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
2007-06-18 21:27 ` Stephen Hemminger [this message]
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=20070618142713.4d030a42@localhost.localdomain \
--to=shemminger@linux-foundation.org \
--cc=greg@kroah.com \
--cc=linux-kernel@vger.kernel.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 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.