Patchwork Fix gcc-4.5.2 "statement with no effect" warnings in UBIFS

login
register
mail settings
Submitter maksim.rayskiy@gmail.com
Date April 11, 2011, 9:50 p.m.
Message ID <1302558659-29472-1-git-send-email-maksim.rayskiy@gmail.com>
Download mbox | patch
Permalink /patch/90660/
State New
Headers show

Comments

maksim.rayskiy@gmail.com - April 11, 2011, 9:50 p.m.
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(-)
Mike Frysinger - April 11, 2011, 10:08 p.m.
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
maksim.rayskiy@gmail.com - April 11, 2011, 10:36 p.m.
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.
Russ Dill - April 12, 2011, 12:12 a.m.
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.
Russ Dill - April 12, 2011, 12:27 a.m.
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
Mike Frysinger - April 12, 2011, 12:49 a.m.
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
Artem Bityutskiy - April 12, 2011, 8:25 a.m.
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?

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__ */