Messages in this thread Patch in this message | | | Date | Mon, 18 Jun 2007 13:41:32 -0700 | From | Stephen Hemminger <> | Subject | [RFC] debugfs: create file error handling |
| |
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |