| Message ID | 1302558659-29472-1-git-send-email-maksim.rayskiy@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 11, 2011 at 17:50, <maksim.rayskiy@gmail.com> wrote: > From: Maksim Rayskiy <maksim.rayskiy@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; }) we probably want to make these inline funcs. otherwise we get misbehavior if someone does something like: dbg_dump_index(c++); -mike
On Mon, Apr 11, 2011 at 3:08 PM, Mike Frysinger <vapier.adi@gmail.com> wrote: > On Mon, Apr 11, 2011 at 17:50, <maksim.rayskiy@gmail.com> wrote: >> From: Maksim Rayskiy <maksim.rayskiy@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; }) > > we probably want to make these inline funcs. otherwise we get > misbehavior if someone does something like: > 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.
On Mon, Apr 11, 2011 at 2:50 PM, <maksim.rayskiy@gmail.com> wrote: > From: Maksim Rayskiy <maksim.rayskiy@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@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.
On Mon, Apr 11, 2011 at 5:12 PM, Russ Dill <russ.dill@gmail.com> wrote: > On Mon, Apr 11, 2011 at 2:50 PM, <maksim.rayskiy@gmail.com> wrote: >> From: Maksim Rayskiy <maksim.rayskiy@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@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) never-mind, I see while the ({0;}) format is preferable. > Stuff like: > > #define dbg_debugfs_exit() > > looks particularly dangerous. but this still stands
On Mon, Apr 11, 2011 at 18:36, Maksim Rayskiy wrote: > On Mon, Apr 11, 2011 at 3:08 PM, Mike Frysinger wrote: >> On Mon, Apr 11, 2011 at 17:50, <maksim.rayskiy@gmail.com> wrote: >>> From: Maksim Rayskiy <maksim.rayskiy@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; }) >> >> we probably want to make these inline funcs. otherwise we get >> misbehavior if someone does something like: >> dbg_dump_index(c++); > > 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. we are dealing with broken code. people should not have to know about the implementation of functions in order to safely use them. -mike
On Mon, 2011-04-11 at 14:50 -0700, maksim.rayskiy@gmail.com wrote: > From: Maksim Rayskiy <maksim.rayskiy@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; }) 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?
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__ */