| Message ID | 20170511124932.226016-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 5/11/17 7:49 AM, Arnd Bergmann wrote: > gcc-7 flags the use of integer math inside of a condition > as a potential bug: > > fs/xfs/xfs_bmap_util.c: In function 'xfs_swap_extents_check_format': > fs/xfs/xfs_bmap_util.c:1619:8: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context] > fs/xfs/xfs_bmap_util.c:1629:8: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context] > > This one is clearly fine, and we can add a comparison to zero > to shut up the warning. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Thanks Arnd - Reviewed-by: Eric Sandeen <sandeen@redhat.com> > --- > fs/xfs/xfs_bmap_util.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > index 2b954308a1d6..cbd3ffe42f39 100644 > --- a/fs/xfs/xfs_bmap_util.c > +++ b/fs/xfs/xfs_bmap_util.c > @@ -1613,7 +1613,7 @@ xfs_swap_extents_check_format( > * extent format... > */ > if (tip->i_d.di_format == XFS_DINODE_FMT_BTREE) { > - if (XFS_IFORK_BOFF(ip) && > + if ((XFS_IFORK_BOFF(ip) != 0) && > XFS_BMAP_BMDR_SPACE(tip->i_df.if_broot) > XFS_IFORK_BOFF(ip)) > return -EINVAL; > if (XFS_IFORK_NEXTENTS(tip, XFS_DATA_FORK) <= > @@ -1623,7 +1623,7 @@ xfs_swap_extents_check_format( > > /* Reciprocal target->temp btree format checks */ > if (ip->i_d.di_format == XFS_DINODE_FMT_BTREE) { > - if (XFS_IFORK_BOFF(tip) && > + if ((XFS_IFORK_BOFF(tip) != 0) && > XFS_BMAP_BMDR_SPACE(ip->i_df.if_broot) > XFS_IFORK_BOFF(tip)) > return -EINVAL; > if (XFS_IFORK_NEXTENTS(ip, XFS_DATA_FORK) <= > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 11, 2017 at 02:49:21PM +0200, Arnd Bergmann wrote: > gcc-7 flags the use of integer math inside of a condition > as a potential bug: > > fs/xfs/xfs_bmap_util.c: In function 'xfs_swap_extents_check_format': > fs/xfs/xfs_bmap_util.c:1619:8: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context] > fs/xfs/xfs_bmap_util.c:1629:8: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context] > > This one is clearly fine, and we can add a comparison to zero > to shut up the warning. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > fs/xfs/xfs_bmap_util.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c > index 2b954308a1d6..cbd3ffe42f39 100644 > --- a/fs/xfs/xfs_bmap_util.c > +++ b/fs/xfs/xfs_bmap_util.c > @@ -1613,7 +1613,7 @@ xfs_swap_extents_check_format( > * extent format... > */ > if (tip->i_d.di_format == XFS_DINODE_FMT_BTREE) { > - if (XFS_IFORK_BOFF(ip) && > + if ((XFS_IFORK_BOFF(ip) != 0) && Even if we were fine with fixing this odd warning the additional braces are simply bogus. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 11, 2017 at 4:02 PM, Christoph Hellwig <hch@infradead.org> wrote: > On Thu, May 11, 2017 at 02:49:21PM +0200, Arnd Bergmann wrote: >> gcc-7 flags the use of integer math inside of a condition >> as a potential bug: >> >> fs/xfs/xfs_bmap_util.c: In function 'xfs_swap_extents_check_format': >> fs/xfs/xfs_bmap_util.c:1619:8: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context] >> fs/xfs/xfs_bmap_util.c:1629:8: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context] >> >> This one is clearly fine, and we can add a comparison to zero >> to shut up the warning. >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- >> fs/xfs/xfs_bmap_util.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c >> index 2b954308a1d6..cbd3ffe42f39 100644 >> --- a/fs/xfs/xfs_bmap_util.c >> +++ b/fs/xfs/xfs_bmap_util.c >> @@ -1613,7 +1613,7 @@ xfs_swap_extents_check_format( >> * extent format... >> */ >> if (tip->i_d.di_format == XFS_DINODE_FMT_BTREE) { >> - if (XFS_IFORK_BOFF(ip) && >> + if ((XFS_IFORK_BOFF(ip) != 0) && > > Even if we were fine with fixing this odd warning the additional braces > are simply bogus. The warning seems generally useful and is enabled by default in gcc-7. An example of a real bug it found is https://patchwork.kernel.org/patch/9431813/, so I'd prefer to leave it enabled and fix the few instances in the kernel. I found a better way to rework the code to avoid the warning, sending out v2 now. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2017-05-11 at 14:49 +0200, Arnd Bergmann wrote: > gcc-7 flags the use of integer math inside of a condition > as a potential bug: > > fs/xfs/xfs_bmap_util.c: In function 'xfs_swap_extents_check_format': > fs/xfs/xfs_bmap_util.c:1619:8: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context] > fs/xfs/xfs_bmap_util.c:1629:8: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context] > > This one is clearly fine, and we can add a comparison to zero > to shut up the warning. [] > diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c [] > @@ -1613,7 +1613,7 @@ xfs_swap_extents_check_format( > * extent format... > */ > if (tip->i_d.di_format == XFS_DINODE_FMT_BTREE) { > - if (XFS_IFORK_BOFF(ip) && > + if ((XFS_IFORK_BOFF(ip) != 0) && As far as I can tell, this suggestion makes no sense. $ git grep -E "define\s+XFS_IFORK_BOFF" fs/xfs/libxfs/xfs_inode_fork.h:#define XFS_IFORK_BOFF(ip) ((int)((ip)->i_d.di_forkoff << 3)) $ git grep -w di_forkoff|grep -P "\w+\s+di_forkoff\s*;" fs/xfs/libxfs/xfs_format.h: __u8 di_forkoff; /* attr fork offs, <<3 for 64b align */ fs/xfs/libxfs/xfs_inode_buf.h: __uint8_t di_forkoff; /* attr fork offs, <<3 for 64b align */ fs/xfs/libxfs/xfs_log_format.h: __uint8_t di_forkoff; /* attr fork offs, <<3 for 64b align */ -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c index 2b954308a1d6..cbd3ffe42f39 100644 --- a/fs/xfs/xfs_bmap_util.c +++ b/fs/xfs/xfs_bmap_util.c @@ -1613,7 +1613,7 @@ xfs_swap_extents_check_format( * extent format... */ if (tip->i_d.di_format == XFS_DINODE_FMT_BTREE) { - if (XFS_IFORK_BOFF(ip) && + if ((XFS_IFORK_BOFF(ip) != 0) && XFS_BMAP_BMDR_SPACE(tip->i_df.if_broot) > XFS_IFORK_BOFF(ip)) return -EINVAL; if (XFS_IFORK_NEXTENTS(tip, XFS_DATA_FORK) <= @@ -1623,7 +1623,7 @@ xfs_swap_extents_check_format( /* Reciprocal target->temp btree format checks */ if (ip->i_d.di_format == XFS_DINODE_FMT_BTREE) { - if (XFS_IFORK_BOFF(tip) && + if ((XFS_IFORK_BOFF(tip) != 0) && XFS_BMAP_BMDR_SPACE(ip->i_df.if_broot) > XFS_IFORK_BOFF(tip)) return -EINVAL; if (XFS_IFORK_NEXTENTS(ip, XFS_DATA_FORK) <=
gcc-7 flags the use of integer math inside of a condition as a potential bug: fs/xfs/xfs_bmap_util.c: In function 'xfs_swap_extents_check_format': fs/xfs/xfs_bmap_util.c:1619:8: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context] fs/xfs/xfs_bmap_util.c:1629:8: error: '<<' in boolean context, did you mean '<' ? [-Werror=int-in-bool-context] This one is clearly fine, and we can add a comparison to zero to shut up the warning. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/xfs/xfs_bmap_util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)