[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20070618205727.GA14857@kroah.com>
Date: Mon, 18 Jun 2007 13:57:27 -0700
From: Greg KH <greg@...ah.com>
To: Stephen Hemminger <shemminger@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [RFC] debugfs: create file error handling
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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists