[RFC] debugfs: create file error handling

From: Stephen Hemminger
Date: Mon Jun 18 2007 - 16:41:49 EST


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().
*/
struct dentry *debugfs_create_u8(const char *name, mode_t mode,
struct dentry *parent, u8 *value)
@@ -124,12 +123,11 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u16, debugf
* 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().
*/
struct dentry *debugfs_create_u16(const char *name, mode_t mode,
struct dentry *parent, u16 *value)
@@ -165,12 +163,11 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u32, debugf
* 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().
*/
struct dentry *debugfs_create_u32(const char *name, mode_t mode,
struct dentry *parent, u32 *value)
@@ -207,12 +204,11 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_u64, debugf
* 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().
*/
struct dentry *debugfs_create_u64(const char *name, mode_t mode,
struct dentry *parent, u64 *value)
@@ -286,12 +282,11 @@ static const struct file_operations fops
* 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().
*/
struct dentry *debugfs_create_bool(const char *name, mode_t mode,
struct dentry *parent, u32 *value)
@@ -330,12 +325,12 @@ static const struct file_operations fops
* 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().
*/
struct dentry *debugfs_create_blob(const char *name, mode_t mode,
struct dentry *parent,
--- a/fs/debugfs/inode.c 2007-05-30 10:32:32.000000000 -0700
+++ b/fs/debugfs/inode.c 2007-06-18 13:38:26.000000000 -0700
@@ -226,14 +226,11 @@ struct dentry *debugfs_create_file(const
error = simple_pin_fs(&debug_fs_type, &debugfs_mount,
&debugfs_mount_count);
if (error)
- goto exit;
+ goto err;

error = debugfs_create_by_name(name, mode, parent, &dentry);
- if (error) {
- dentry = NULL;
- simple_release_fs(&debugfs_mount, &debugfs_mount_count);
- goto exit;
- }
+ if (error)
+ goto release;

if (dentry->d_inode) {
if (data)
@@ -241,8 +238,12 @@ struct dentry *debugfs_create_file(const
if (fops)
dentry->d_inode->i_fop = fops;
}
-exit:
+
return dentry;
+release:
+ simple_release_fs(&debugfs_mount, &debugfs_mount_count);
+err:
+ return ERR_PTR(error);
}
EXPORT_SYMBOL_GPL(debugfs_create_file);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/