ubifs: no need to check return value of debugfs_create functions
Subsystems (2)
PATCH 1/1 ubifs: no need to check return value of debugfs_create functions
When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this.
Cc Richard Weinberger <[email protected]>
Cc Artem Bityutskiy <[email protected]>
Cc Adrian Hunter <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
--- fs/ubifs/debug.c | 167 ++++++++++++----------------------------------- fs/ubifs/debug.h | 4 +- fs/ubifs/super.c | 9 +-- 3 files changed, 45 insertions(+), 135 deletions(-)
fs/ubifs/debug.c +41 -126
@@ -2812,115 +2812,69 @@ static const struct file_operations dfs_fops = {* dbg_debugfs_init_fs - initialize debugfs for UBIFS instance. * @c: UBIFS file-system description object * -* This function creates all debugfs files for this instance of UBIFS. Returns -* zero in case of success and a negative error code in case of failure. +* This function creates all debugfs files for this instance of UBIFS. * * Note, the only reason we have not merged this function with the * 'ubifs_debugging_init()' function is because it is better to initialize * debugfs interfaces at the very end of the mount process, and remove them at * the very beginning of the mount process. */ -int dbg_debugfs_init_fs (struct ubifs_info *c) +void dbg_debugfs_init_fs (struct ubifs_info *c) {int err, n;const char *fname; -struct dentry *dent;struct ubifs_debug_info *d = c->dbg; -if (!IS_ENABLED (CONFIG_DEBUG_FS )) -return 0 ; - n =snprintf (d->dfs_dir_name,UBIFS_DFS_DIR_LEN +1 ,UBIFS_DFS_DIR_NAME , c->vi.ubi_num, c->vi.vol_id);if (n ==UBIFS_DFS_DIR_LEN ) {/* The array size is too small */ fname =UBIFS_DFS_DIR_NAME ; - dent =ERR_PTR (-EINVAL ); - goto out; +return ; } fname = d->dfs_dir_name; - dent =debugfs_create_dir (fname, dfs_rootdir); -if (IS_ERR_OR_NULL (dent)) - goto out; - d->dfs_dir = dent; + d->dfs_dir =debugfs_create_dir (fname, dfs_rootdir); fname ="dump_lprops" ; - dent =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_dump_lprops = dent; + d->dfs_dump_lprops =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, + &dfs_fops); fname ="dump_budg" ; - dent =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_dump_budg = dent; + d->dfs_dump_budg =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, + &dfs_fops); fname ="dump_tnc" ; - dent =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_dump_tnc = dent; + d->dfs_dump_tnc =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, + &dfs_fops); fname ="chk_general" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_chk_gen = dent; + d->dfs_chk_gen =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="chk_index" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_chk_index = dent; + d->dfs_chk_index =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="chk_orphans" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_chk_orph = dent; + d->dfs_chk_orph =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="chk_lprops" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_chk_lprops = dent; + d->dfs_chk_lprops =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="chk_fs" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_chk_fs = dent; + d->dfs_chk_fs =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="tst_recovery" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_tst_rcvry = dent; + d->dfs_tst_rcvry =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="ro_error" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_ro_error = dent; - -return 0 ; - -out_remove: -debugfs_remove_recursive (d->dfs_dir); -out: - err = dent ?PTR_ERR (dent) : -ENODEV ; -ubifs_err (c,"cannot create \"%s\" debugfs file or directory, error %d\n" , - fname, err); -return err; + d->dfs_ro_error =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); }/**
@@ -2929,8 +2883,7 @@ int dbg_debugfs_init_fs(struct ubifs_info *c)*/ void dbg_debugfs_exit_fs (struct ubifs_info *c) { -if (IS_ENABLED (CONFIG_DEBUG_FS )) -debugfs_remove_recursive (c->dbg->dfs_dir); +debugfs_remove_recursive (c->dbg->dfs_dir); }struct ubifs_global_debug_info ubifs_dbg;
@@ -3006,75 +2959,38 @@ static const struct file_operations dfs_global_fops = {* * UBIFS uses debugfs file-system to expose various debugging knobs to * user-space. This function creates "ubifs" directory in the debugfs -* file-system. Returns zero in case of success and a negative error code in -* case of failure. +* file-system. */ -int dbg_debugfs_init (void ) +void dbg_debugfs_init (void ) { -int err;const char *fname; -struct dentry *dent; - -if (!IS_ENABLED (CONFIG_DEBUG_FS )) -return 0 ; fname ="ubifs" ; - dent =debugfs_create_dir (fname,NULL ); -if (IS_ERR_OR_NULL (dent)) - goto out; - dfs_rootdir = dent; + dfs_rootdir =debugfs_create_dir (fname,NULL ); fname ="chk_general" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_chk_gen = dent; + dfs_chk_gen =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir, +NULL , &dfs_global_fops); fname ="chk_index" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_chk_index = dent; + dfs_chk_index =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + dfs_rootdir,NULL , &dfs_global_fops); fname ="chk_orphans" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_chk_orph = dent; + dfs_chk_orph =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + dfs_rootdir,NULL , &dfs_global_fops); fname ="chk_lprops" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_chk_lprops = dent; + dfs_chk_lprops =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + dfs_rootdir,NULL , &dfs_global_fops); fname ="chk_fs" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_chk_fs = dent; + dfs_chk_fs =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir, +NULL , &dfs_global_fops); fname ="tst_recovery" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_tst_rcvry = dent; - -return 0 ; - -out_remove: -debugfs_remove_recursive (dfs_rootdir); -out: - err = dent ?PTR_ERR (dent) : -ENODEV ; -pr_err ("UBIFS error (pid %d): cannot create \"%s\" debugfs file or directory, error %d\n" , - current->pid, fname, err); -return err; + dfs_tst_rcvry =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + dfs_rootdir,NULL , &dfs_global_fops); }/**
@@ -3082,8 +2998,7 @@ int dbg_debugfs_init(void)*/ void dbg_debugfs_exit (void ) { -if (IS_ENABLED (CONFIG_DEBUG_FS )) -debugfs_remove_recursive (dfs_rootdir); +debugfs_remove_recursive (dfs_rootdir); } void ubifs_assert_failed(struct ubifs_info *c, const char *expr,
fs/ubifs/debug.h +2 -2
@@ -309,9 +309,9 @@ int dbg_leb_unmap(struct ubifs_info *c, int lnum);int dbg_leb_map (struct ubifs_info *c,int lnum);/* Debugfs-related stuff */ -int dbg_debugfs_init (void ); +void dbg_debugfs_init (void );void dbg_debugfs_exit (void ); -int dbg_debugfs_init_fs (struct ubifs_info *c); +void dbg_debugfs_init_fs (struct ubifs_info *c);void dbg_debugfs_exit_fs (struct ubifs_info *c);#endif /* !__UBIFS_DEBUG_H__ */
fs/ubifs/super.c +2 -7
@@ -1477,9 +1477,7 @@ static int mount_ubifs(struct ubifs_info *c)if (err) goto out_infos; - err =dbg_debugfs_init_fs (c); -if (err) - goto out_infos; +dbg_debugfs_init_fs (c); c->mounting =0 ;
@@ -2364,9 +2362,7 @@ static int __init ubifs_init(void)if (err) goto out_shrinker; - err =dbg_debugfs_init (); -if (err) - goto out_compr; +dbg_debugfs_init (); err =register_filesystem (&ubifs_fs_type);if (err) {
@@ -2378,7 +2374,6 @@ static int __init ubifs_init(void) out_dbg:dbg_debugfs_exit (); -out_compr:ubifs_compressors_exit (); out_shrinker:unregister_shrinker (&ubifs_shrinker_info);
Signature · 6 lines
2.22.0 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
I think you should still abort if the directory fails to be created. If it's NULL, all later file creation will be attempted at root debugfs. Not only will they pollute the filesystem, but since they are not attached to the directory we remove on cleanup, they'll leak. Also, since failure is no longer reported, it may be a good idea to log that failure.
Quoted 325 lines · Greg Kroah-Hartman
When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this.
Cc Richard Weinberger <[email protected]>Cc Artem Bityutskiy <[email protected]>Cc Adrian Hunter <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>--- fs/ubifs/debug.c | 167 ++++++++++++----------------------------------- fs/ubifs/debug.h | 4 +- fs/ubifs/super.c | 9 +-- 3 files changed, 45 insertions(+), 135 deletions(-)fs/ubifs/debug.c +41
-126@@ -2812,115 +2812,69 @@ static const struct file_operations dfs_fops = {* dbg_debugfs_init_fs - initialize debugfs for UBIFS instance. * @c: UBIFS file-system description object * -* This function creates all debugfs files for this instance of UBIFS. Returns -* zero in case of success and a negative error code in case of failure. +* This function creates all debugfs files for this instance of UBIFS. * * Note, the only reason we have not merged this function with the * 'ubifs_debugging_init()' function is because it is better to initialize * debugfs interfaces at the very end of the mount process, and remove them at * the very beginning of the mount process. */ -int dbg_debugfs_init_fs (struct ubifs_info *c) +void dbg_debugfs_init_fs (struct ubifs_info *c) {int err, n;const char *fname; -struct dentry *dent;struct ubifs_debug_info *d = c->dbg; -if (!IS_ENABLED (CONFIG_DEBUG_FS )) -return 0 ; - n =snprintf (d->dfs_dir_name,UBIFS_DFS_DIR_LEN +1 ,UBIFS_DFS_DIR_NAME , c->vi.ubi_num, c->vi.vol_id);if (n ==UBIFS_DFS_DIR_LEN ) {/* The array size is too small */ fname =UBIFS_DFS_DIR_NAME ; - dent =ERR_PTR (-EINVAL ); - goto out; +return ; } fname = d->dfs_dir_name; - dent =debugfs_create_dir (fname, dfs_rootdir); -if (IS_ERR_OR_NULL (dent)) - goto out; - d->dfs_dir = dent; + d->dfs_dir =debugfs_create_dir (fname, dfs_rootdir); fname ="dump_lprops" ; - dent =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_dump_lprops = dent; + d->dfs_dump_lprops =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, + &dfs_fops); fname ="dump_budg" ; - dent =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_dump_budg = dent; + d->dfs_dump_budg =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, + &dfs_fops); fname ="dump_tnc" ; - dent =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_dump_tnc = dent; + d->dfs_dump_tnc =debugfs_create_file (fname,S_IWUSR , d->dfs_dir, c, + &dfs_fops); fname ="chk_general" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_chk_gen = dent; + d->dfs_chk_gen =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="chk_index" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_chk_index = dent; + d->dfs_chk_index =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="chk_orphans" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_chk_orph = dent; + d->dfs_chk_orph =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="chk_lprops" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_chk_lprops = dent; + d->dfs_chk_lprops =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="chk_fs" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_chk_fs = dent; + d->dfs_chk_fs =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="tst_recovery" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_tst_rcvry = dent; + d->dfs_tst_rcvry =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); fname ="ro_error" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , d->dfs_dir, c, - &dfs_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - d->dfs_ro_error = dent; - -return 0 ; - -out_remove: -debugfs_remove_recursive (d->dfs_dir); -out: - err = dent ?PTR_ERR (dent) : -ENODEV ; -ubifs_err (c,"cannot create \"%s\" debugfs file or directory, error %d\n" , - fname, err); -return err; + d->dfs_ro_error =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + d->dfs_dir, c, &dfs_fops); }/** @@ -2929,8 +2883,7 @@ int dbg_debugfs_init_fs(struct ubifs_info *c)*/ void dbg_debugfs_exit_fs (struct ubifs_info *c) { -if (IS_ENABLED (CONFIG_DEBUG_FS )) -debugfs_remove_recursive (c->dbg->dfs_dir); +debugfs_remove_recursive (c->dbg->dfs_dir); }struct ubifs_global_debug_info ubifs_dbg;@@ -3006,75 +2959,38 @@ static const struct file_operations dfs_global_fops = {* * UBIFS uses debugfs file-system to expose various debugging knobs to * user-space. This function creates "ubifs" directory in the debugfs -* file-system. Returns zero in case of success and a negative error code in -* case of failure. +* file-system. */ -int dbg_debugfs_init (void ) +void dbg_debugfs_init (void ) { -int err;const char *fname; -struct dentry *dent; - -if (!IS_ENABLED (CONFIG_DEBUG_FS )) -return 0 ; fname ="ubifs" ; - dent =debugfs_create_dir (fname,NULL ); -if (IS_ERR_OR_NULL (dent)) - goto out; - dfs_rootdir = dent; + dfs_rootdir =debugfs_create_dir (fname,NULL ); fname ="chk_general" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_chk_gen = dent; + dfs_chk_gen =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir, +NULL , &dfs_global_fops); fname ="chk_index" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_chk_index = dent; + dfs_chk_index =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + dfs_rootdir,NULL , &dfs_global_fops); fname ="chk_orphans" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_chk_orph = dent; + dfs_chk_orph =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + dfs_rootdir,NULL , &dfs_global_fops); fname ="chk_lprops" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_chk_lprops = dent; + dfs_chk_lprops =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + dfs_rootdir,NULL , &dfs_global_fops); fname ="chk_fs" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_chk_fs = dent; + dfs_chk_fs =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir, +NULL , &dfs_global_fops); fname ="tst_recovery" ; - dent =debugfs_create_file (fname,S_IRUSR |S_IWUSR , dfs_rootdir,NULL , - &dfs_global_fops); -if (IS_ERR_OR_NULL (dent)) - goto out_remove; - dfs_tst_rcvry = dent; - -return 0 ; - -out_remove: -debugfs_remove_recursive (dfs_rootdir); -out: - err = dent ?PTR_ERR (dent) : -ENODEV ; -pr_err ("UBIFS error (pid %d): cannot create \"%s\" debugfs file or directory, error %d\n" , - current->pid, fname, err); -return err; + dfs_tst_rcvry =debugfs_create_file (fname,S_IRUSR |S_IWUSR , + dfs_rootdir,NULL , &dfs_global_fops); }/** @@ -3082,8 +2998,7 @@ int dbg_debugfs_init(void)*/ void dbg_debugfs_exit (void ) { -if (IS_ENABLED (CONFIG_DEBUG_FS )) -debugfs_remove_recursive (dfs_rootdir); +debugfs_remove_recursive (dfs_rootdir); } void ubifs_assert_failed(struct ubifs_info *c, const char *expr,fs/ubifs/debug.h +2
-2@@ -309,9 +309,9 @@ int dbg_leb_unmap(struct ubifs_info *c, int lnum);int dbg_leb_map (struct ubifs_info *c,int lnum);/* Debugfs-related stuff */ -int dbg_debugfs_init (void ); +void dbg_debugfs_init (void );void dbg_debugfs_exit (void ); -int dbg_debugfs_init_fs (struct ubifs_info *c); +void dbg_debugfs_init_fs (struct ubifs_info *c);void dbg_debugfs_exit_fs (struct ubifs_info *c);#endif /* !__UBIFS_DEBUG_H__ */ fs/ubifs/super.c +2
-8@@ -1477,9 +1477,7 @@ static int mount_ubifs(struct ubifs_info *c)if (err) goto out_infos; - err =dbg_debugfs_init_fs (c); -if (err) - goto out_infos; +dbg_debugfs_init_fs (c); c->mounting =0 ;@@ -2364,9 +2362,7 @@ static int __init ubifs_init(void)if (err) goto out_shrinker; - err =dbg_debugfs_init (); -if (err) - goto out_compr; +dbg_debugfs_init (); err =register_filesystem (&ubifs_fs_type);if (err) {@@ -2378,7 +2374,6 @@ static int __init ubifs_init(void) out_dbg:dbg_debugfs_exit (); -out_compr:ubifs_compressors_exit (); out_shrinker:unregister_shrinker (&ubifs_shrinker_info); --2.22.0
Quoted 3 lines
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
A: Top-posting.
Q: What is the most annoying thing in e-mail?
A: No.
Q: Should I include quotations after my reply?
http://daringfireball.net/2007/07/on_top
Quoted 1 line · Mario Rugiero
I think you should still abort if the directory fails to be created.
Why?
Quoted 1 line
If it's NULL, all later file creation will be attempted at root debugfs.
debugfs_create_dir() can not return NULL.
Quoted 2 lines
Not only will they pollute the filesystem, but since they are not attached to the directory we remove on cleanup, they'll leak.
That can not happen, if the parent directory is an error pointer, the file will just not be created.
Quoted 2 lines
Also, since failure is no longer reported, it may be a good idea to log that failure.
Why? To where? For what? debugfs should never matter if it works or not as no no-debugging userspace code should ever depend on it, and the kernel should not ever "fail" anything if it is not working properly.
thanks,
greg k-h
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Richard Weinberger
maintainerGreg,
----- Ursprüngliche Mail -----
Quoted 9 lines
Von: "Greg Kroah-Hartman" <[email protected]>
An: "richard" <[email protected]>, "Artem Bityutskiy" <[email protected]>, "Adrian Hunter" <[email protected]>
CC: "linux-mtd" <[email protected]>
Gesendet: Mittwoch, 12. Juni 2019 17:21:20
Betreff: [PATCH] ubifs: no need to check return value ofdebugfs_createfunctionsWhen calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this.
why is it bad if we handle errors from debugfs?
At least a nice log message a la "sorry, no ubifs debuging for you" would be nice to have.
Thanks,
//richard
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Quoted 9 lines · Richard Weinberger
Greg,
----- Ursprüngliche Mail -----
Quoted 5 lines
Von: "Greg Kroah-Hartman" <[email protected]>
An: "richard" <[email protected]>, "Artem Bityutskiy" <[email protected]>, "Adrian Hunter" <[email protected]>
CC: "linux-mtd" <[email protected]>
Gesendet: Mittwoch, 12. Juni 2019 17:21:20
Betreff: [PATCH] ubifs: no need to check return value ofdebugfs_createfunctions
Quoted 5 lines
Quoted 3 lines
When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this.
why is it bad if we handle errors from debugfs?
Because there is no need to? If debugfs fails, there is nothing that a user can do about it :)
Quoted 2 lines
At least a nice log message a la "sorry, no ubifs debuging for you" would be nice to have.
I think the lack of a debugfs file is kind of obvious :)
Anyway, the point is that there is no need to check for return values from debugfs, and the logic of the non-debugfs code should not do anything different if debugfs is acting up or not.
thanks,
greg k-h
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/