[PATCH] Fix gcc-4.5.2 "statement with no effect" warnings in UBIFS

Discussion:

maksim.rayskiy

2011-04-11 21:50:59 UTC

From: Maksim Rayskiy <maksim.rayskiy at gmail.com>

When compiling UBIFS with CONFIG_UBIFS_FS_DEBUG not set,
gcc-4.5.2 generates a slew of "warning: statement with no effect"
on references to non-void functions defined as 0.
To avoid these warnings, change appropriate definitions from
#define dbg_xxx(a) 0
to
#define dbg_xxx(a) ({0; })

Signed-off-by: Maksim Rayskiy <maksim.rayskiy at gmail.com>
---
fs/ubifs/debug.h | 50 +++++++++++++++++++++++++-------------------------
1 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/fs/ubifs/debug.h b/fs/ubifs/debug.h
index 919f0de..e30a629 100644
--- a/fs/ubifs/debug.h
+++ b/fs/ubifs/debug.h
@@ -425,34 +425,34 @@ void dbg_debugfs_exit_fs(struct ubifs_info *c);
#define dbg_dump_index(c) ({})
#define dbg_dump_lpt_lebs(c) ({})

-#define dbg_walk_index(c, leaf_cb, znode_cb, priv) 0
-#define dbg_old_index_check_init(c, zroot) 0
+#define dbg_walk_index(c, leaf_cb, znode_cb, priv) ({0; })
+#define dbg_old_index_check_init(c, zroot) ({0; })
#define dbg_save_space_info(c) ({})
-#define dbg_check_space_info(c) 0
-#define dbg_check_old_index(c, zroot) 0
-#define dbg_check_cats(c) 0
-#define dbg_check_ltab(c) 0
-#define dbg_chk_lpt_free_spc(c) 0
-#define dbg_chk_lpt_sz(c, action, len) 0
-#define dbg_check_synced_i_size(inode) 0
-#define dbg_check_dir_size(c, dir) 0
-#define dbg_check_tnc(c, x) 0
-#define dbg_check_idx_size(c, idx_size) 0
-#define dbg_check_filesystem(c) 0
+#define dbg_check_space_info(c) ({0; })
+#define dbg_check_old_index(c, zroot) ({0; })
+#define dbg_check_cats(c) ({0; })
+#define dbg_check_ltab(c) ({0; })
+#define dbg_chk_lpt_free_spc(c) ({0; })
+#define dbg_chk_lpt_sz(c, action, len) ({0; })
+#define dbg_check_synced_i_size(inode) ({0; })
+#define dbg_check_dir_size(c, dir) ({0; })
+#define dbg_check_tnc(c, x) ({0; })
+#define dbg_check_idx_size(c, idx_size) ({0; })
+#define dbg_check_filesystem(c) ({0; })
#define dbg_check_heap(c, heap, cat, add_pos) ({})
-#define dbg_check_lprops(c) 0
-#define dbg_check_lpt_nodes(c, cnode, row, col) 0
-#define dbg_check_inode_size(c, inode, size) 0
-#define dbg_check_data_nodes_order(c, head) 0
-#define dbg_check_nondata_nodes_order(c, head) 0
-#define dbg_force_in_the_gaps_enabled 0
-#define dbg_force_in_the_gaps() 0
-#define dbg_failure_mode 0
-
-#define dbg_debugfs_init() 0
+#define dbg_check_lprops(c) ({0; })
+#define dbg_check_lpt_nodes(c, cnode, row, col) ({0; })
+#define dbg_check_inode_size(c, inode, size) ({0; })
+#define dbg_check_data_nodes_order(c, head) ({0; })
+#define dbg_check_nondata_nodes_order(c, head) ({0; })
+#define dbg_force_in_the_gaps_enabled ({0; })
+#define dbg_force_in_the_gaps() ({0; })
+#define dbg_failure_mode ({0; })
+
+#define dbg_debugfs_init() ({0; })
#define dbg_debugfs_exit()
-#define dbg_debugfs_init_fs(c) 0
-#define dbg_debugfs_exit_fs(c) 0
+#define dbg_debugfs_init_fs(c) ({0; })
+#define dbg_debugfs_exit_fs(c) ({0; })

#endif /* !CONFIG_UBIFS_FS_DEBUG */
#endif /* !__UBIFS_DEBUG_H__ */

--
1.7.1

Mike Frysinger

2011-04-11 22:08:48 UTC

Permalink

we probably want to make these inline funcs. otherwise we get
misbehavior if someone does something like:
dbg_dump_index(c++);
-mike

Maksim Rayskiy

2011-04-11 22:36:55 UTC

Permalink

we probably want to make these inline funcs. ?otherwise we get
? ?dbg_dump_index(c++);
-mike

I agree that would be a problem, but if such calls existed, we would
be dealing not with compiler warnings but with completely broken code.
I think it is bad style to have side-effects in debug functions.

Thanks,
Maksim.

Mike Frysinger

2011-04-12 00:49:53 UTC

Permalink

Post by Maksim Rayskiy

we probably want to make these inline funcs. ?otherwise we get
? ?dbg_dump_index(c++);

we are dealing with broken code. people should not have to know about
the implementation of functions in order to safely use them.
-mike

Russ Dill

2011-04-12 00:12:14 UTC

Permalink

Post by maksim.rayskiy
From: Maksim Rayskiy <maksim.rayskiy at gmail.com>
When compiling UBIFS with CONFIG_UBIFS_FS_DEBUG not set,
gcc-4.5.2 generates a slew of "warning: statement with no effect"
on references to non-void functions defined as 0.
To avoid these warnings, change appropriate definitions from
?#define dbg_xxx(a) ? ? 0
to
?#define dbg_xxx(a) ? ? ({0; })
Signed-off-by: Maksim Rayskiy <maksim.rayskiy at gmail.com>
---
?fs/ubifs/debug.h | ? 50 +++++++++++++++++++++++++-------------------------
?1 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/fs/ubifs/debug.h b/fs/ubifs/debug.h
index 919f0de..e30a629 100644
--- a/fs/ubifs/debug.h
+++ b/fs/ubifs/debug.h
@@ -425,34 +425,34 @@ void dbg_debugfs_exit_fs(struct ubifs_info *c);
?#define dbg_dump_index(c) ? ? ? ? ? ? ? ? ? ? ?({})
?#define dbg_dump_lpt_lebs(c) ? ? ? ? ? ? ? ? ? ({})
-#define dbg_walk_index(c, leaf_cb, znode_cb, priv) 0
-#define dbg_old_index_check_init(c, zroot) ? ? ? ? 0
+#define dbg_walk_index(c, leaf_cb, znode_cb, priv) ({0; })
+#define dbg_old_index_check_init(c, zroot) ? ? ? ? ({0; })
?#define dbg_save_space_info(c) ? ? ? ? ? ? ? ? ? ? ({})
-#define dbg_check_space_info(c) ? ? ? ? ? ? ? ? ? ?0
-#define dbg_check_old_index(c, zroot) ? ? ? ? ? ? ?0
-#define dbg_check_cats(c) ? ? ? ? ? ? ? ? ? ? ? ? ?0
-#define dbg_check_ltab(c) ? ? ? ? ? ? ? ? ? ? ? ? ?0
-#define dbg_chk_lpt_free_spc(c) ? ? ? ? ? ? ? ? ? ?0
-#define dbg_chk_lpt_sz(c, action, len) ? ? ? ? ? ? 0
-#define dbg_check_synced_i_size(inode) ? ? ? ? ? ? 0
-#define dbg_check_dir_size(c, dir) ? ? ? ? ? ? ? ? 0
-#define dbg_check_tnc(c, x) ? ? ? ? ? ? ? ? ? ? ? ?0
-#define dbg_check_idx_size(c, idx_size) ? ? ? ? ? ?0
-#define dbg_check_filesystem(c) ? ? ? ? ? ? ? ? ? ?0
+#define dbg_check_space_info(c) ? ? ? ? ? ? ? ? ? ?({0; })
+#define dbg_check_old_index(c, zroot) ? ? ? ? ? ? ?({0; })
+#define dbg_check_cats(c) ? ? ? ? ? ? ? ? ? ? ? ? ?({0; })
+#define dbg_check_ltab(c) ? ? ? ? ? ? ? ? ? ? ? ? ?({0; })
+#define dbg_chk_lpt_free_spc(c) ? ? ? ? ? ? ? ? ? ?({0; })
+#define dbg_chk_lpt_sz(c, action, len) ? ? ? ? ? ? ({0; })
+#define dbg_check_synced_i_size(inode) ? ? ? ? ? ? ({0; })
+#define dbg_check_dir_size(c, dir) ? ? ? ? ? ? ? ? ({0; })
+#define dbg_check_tnc(c, x) ? ? ? ? ? ? ? ? ? ? ? ?({0; })
+#define dbg_check_idx_size(c, idx_size) ? ? ? ? ? ?({0; })
+#define dbg_check_filesystem(c) ? ? ? ? ? ? ? ? ? ?({0; })
?#define dbg_check_heap(c, heap, cat, add_pos) ? ? ?({})
-#define dbg_check_lprops(c) ? ? ? ? ? ? ? ? ? ? ? ?0
-#define dbg_check_lpt_nodes(c, cnode, row, col) ? ?0
-#define dbg_check_inode_size(c, inode, size) ? ? ? 0
-#define dbg_check_data_nodes_order(c, head) ? ? ? ?0
-#define dbg_check_nondata_nodes_order(c, head) ? ? 0
-#define dbg_force_in_the_gaps_enabled ? ? ? ? ? ? ?0
-#define dbg_force_in_the_gaps() ? ? ? ? ? ? ? ? ? ?0
-#define dbg_failure_mode ? ? ? ? ? ? ? ? ? ? ? ? ? 0
-
-#define dbg_debugfs_init() ? ? ? ? ? ? ? ? ? ? ? ? 0
+#define dbg_check_lprops(c) ? ? ? ? ? ? ? ? ? ? ? ?({0; })
+#define dbg_check_lpt_nodes(c, cnode, row, col) ? ?({0; })
+#define dbg_check_inode_size(c, inode, size) ? ? ? ({0; })
+#define dbg_check_data_nodes_order(c, head) ? ? ? ?({0; })
+#define dbg_check_nondata_nodes_order(c, head) ? ? ({0; })
+#define dbg_force_in_the_gaps_enabled ? ? ? ? ? ? ?({0; })
+#define dbg_force_in_the_gaps() ? ? ? ? ? ? ? ? ? ?({0; })
+#define dbg_failure_mode ? ? ? ? ? ? ? ? ? ? ? ? ? ({0; })
+
+#define dbg_debugfs_init() ? ? ? ? ? ? ? ? ? ? ? ? ({0; })
?#define dbg_debugfs_exit()
-#define dbg_debugfs_init_fs(c) ? ? ? ? ? ? ? ? ? ? 0
-#define dbg_debugfs_exit_fs(c) ? ? ? ? ? ? ? ? ? ? 0
+#define dbg_debugfs_init_fs(c) ? ? ? ? ? ? ? ? ? ? ({0; })
+#define dbg_debugfs_exit_fs(c) ? ? ? ? ? ? ? ? ? ? ({0; })
?#endif /* !CONFIG_UBIFS_FS_DEBUG */
?#endif /* !__UBIFS_DEBUG_H__ */
--
1.7.1

Should these all just be:

do {} while(0)

Stuff like:

#define dbg_debugfs_exit()

looks particularly dangerous.

Russ Dill

2011-04-12 00:27:22 UTC

Permalink

?do {} while(0)

never-mind, I see while the ({0;}) format is preferable.

#define dbg_debugfs_exit()
looks particularly dangerous.

but this still stands

Artem Bityutskiy

2011-04-12 08:25:44 UTC

Permalink

Hi, thanks for the fix, but I agree that something like

static inline int dbg_xxx(a) { return 0; }

would be cleaner. Would you mind trying that approach and sending a
patch?

--
Best Regards,
Artem Bityutskiy (????? ????????)