[PATCH v2] xfs: Silence bounds checking compiler warning

[PATCH v2] xfs: Silence bounds checking compiler warning

[Maarten Lankhorst]

gcc with -Warray-bounds generates a false positive on this
since xfs defines the struct with u8 name[1]; to be able to
add a tag at the end.

Signed-off-by: Maarten Lankhorst <m.b.lankhorst@gmail.com>
---
 fs/xfs/xfs_dir2_block.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/xfs/xfs_dir2_block.c b/fs/xfs/xfs_dir2_block.c
index 580d99c..2c5f287 100644
--- a/fs/xfs/xfs_dir2_block.c
+++ b/fs/xfs/xfs_dir2_block.c
@@ -1148,7 +1148,7 @@ xfs_dir2_sf_to_block(
 		((char *)block + XFS_DIR2_DATA_DOTDOT_OFFSET);
 	dep->inumber = cpu_to_be64(xfs_dir2_sf_get_inumber(sfp, &sfp->hdr.parent));
 	dep->namelen = 2;
-	dep->name[0] = dep->name[1] = '.';
+	memset(dep->name, '.', 2);
 	tagp = xfs_dir2_data_entry_tag_p(dep);
 	*tagp = cpu_to_be16((char *)dep - (char *)block);
 	xfs_dir2_data_log_entry(tp, bp, dep);
-- 
1.7.5.4

v2: memset looks like the cleanest solution, other options rely on ugly casts

Re: [PATCH v2] xfs: Silence bounds checking compiler warning

[Andi Kleen]

Maarten Lankhorst <m.b.lankhorst@gmail.com> writes:

> gcc with -Warray-bounds generates a false positive on this
> since xfs defines the struct with u8 name[1]; to be able to
> add a tag at the end.

A better way would be to define it as name[0]. Then the compiler
would know it's a VLA. You may need to check noone relies on
the one byte though.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH v2] xfs: Silence bounds checking compiler warning

[Al Viro]

On Thu, Jun 23, 2011 at 10:27:26AM -0700, Andi Kleen wrote:
> Maarten Lankhorst <m.b.lankhorst@gmail.com> writes:
> 
> > gcc with -Warray-bounds generates a false positive on this
> > since xfs defines the struct with u8 name[1]; to be able to
> > add a tag at the end.
> 
> A better way would be to define it as name[0]. Then the compiler
> would know it's a VLA. You may need to check noone relies on
> the one byte though.

... and even better is to write in real C and have u8 name[]; in the
end of your structure.  That's the standard C99 for this kind of thing
(see 6.7.2.1p2, p16).  Zero-sized array is a gccism predating standard
flexible array members and since the standard syntax is accepted by
any gcc version that might be recent enough to build the kernel...

Re: [PATCH v2] xfs: Silence bounds checking compiler warning

[Christoph Hellwig]

On Thu, Jun 23, 2011 at 06:55:33PM +0100, Al Viro wrote:
> ... and even better is to write in real C and have u8 name[]; in the
> end of your structure.  That's the standard C99 for this kind of thing
> (see 6.7.2.1p2, p16).  Zero-sized array is a gccism predating standard
> flexible array members and since the standard syntax is accepted by
> any gcc version that might be recent enough to build the kernel...

The situation is even more nasty - the one sized fake flex-array
actually is in the middle of the structure.  Besides sizeof-expressions
taking the one member array into account only members before the
variable sized array are used.  I've started a series cleaning up the
few structures that were done that way (for whatever reason), but it's
pretty intrusive.  I don't think papering over these warnings at this
point is a good idea.

Re: [PATCH v2] xfs: Silence bounds checking compiler warning

[Dave Chinner]

On Thu, Jun 23, 2011 at 06:55:33PM +0100, Al Viro wrote:
> On Thu, Jun 23, 2011 at 10:27:26AM -0700, Andi Kleen wrote:
> > Maarten Lankhorst <m.b.lankhorst@gmail.com> writes:
> > 
> > > gcc with -Warray-bounds generates a false positive on this
> > > since xfs defines the struct with u8 name[1]; to be able to
> > > add a tag at the end.
> > 
> > A better way would be to define it as name[0]. Then the compiler
> > would know it's a VLA. You may need to check noone relies on
> > the one byte though.
> 
> ... and even better is to write in real C and have u8 name[]; in the
> end of your structure.

Hard to do when the structure is effectively the definition of the
on-disk format. Hence it can't just be changed around and the kernel
recompiled to fix the problem.

> That's the standard C99 for this kind of thing
> (see 6.7.2.1p2, p16).  Zero-sized array is a gccism predating standard
> flexible array members and since the standard syntax is accepted by
> any gcc version that might be recent enough to build the kernel...

This code came from Irix, which means it predates both the gccism
and the C99 standard methods of using flexible array sizes. The code
works so it's never been modified because stuffing about with
structures that define disk formats is not done just for the hell of
it... ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com