Skip to contentSkip to conversation

xfsprogs: fix build warnings V2

PATCH In review Dave Chinner 2 participants 19 messages Jan 14, 2010 – Jan 17, 2010

lore
Share & tools
lore
#0 lore raw

cover xfsprogs: fix build warnings V2

This is the complete set of patches that make xfsprogs build cleanly with gcc 4.3.4 on my debian system. The majority of the warnings are dir2 name string sign warnings, most of the rest are array subscript bounds warnings which gcc has got wrong. The only real bug in whole the lot is the missing error check in xfs_rtfree_range().

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw

PATCH 1/9 xfsprogs: fix sign warnings in libxfs

The directory naming is a mix of char and uchar_t and the compiler warns wheter they cross over. Cast the pointers according to the destination requirements as it doesn't seem to matter to avoid the warnings.

---
 include/xfs_types.h     |    2 +-
 libxfs/xfs_attr.c       |   10 +++++-----
 libxfs/xfs_attr_leaf.c  |   10 +++++-----
 libxfs/xfs_dir2.c       |    2 +-
 libxfs/xfs_dir2_block.c |    8 ++++----
 libxfs/xfs_dir2_leaf.c  |    4 ++--
 libxfs/xfs_dir2_node.c  |    4 ++--
 libxfs/xfs_dir2_sf.c    |   13 +++++++------
 8 files changed, 27 insertions(+), 26 deletions(-)
include/xfs_types.h +1 -1
index 0f51916..6f566ff 100644
@@ -161,7 +161,7 @@ typedef enum {
 } xfs_btnum_t;
 
 struct xfs_name {
-	const char	*name;
+	const uchar_t	*name;
 	int		len;
 };
 
libxfs/xfs_attr.c +5 -5
index 7ab37ff..5bd3c2a 100644
@@ -64,7 +64,7 @@ xfs_attr_name_to_xname(
 {
 	if (!aname)
 		return EINVAL;
-	xname->name = aname;
+	xname->name = (const uchar_t *)aname;
 	xname->len = strlen(aname);
 	if (xname->len >= MAXNAMELEN)
 		return EFAULT;		/* match IRIX behaviour */
@@ -103,7 +103,7 @@ xfs_attr_fetch(xfs_inode_t *ip, struct xfs_name *name,
 	memset((char *)&args, 0, sizeof(args));
 	args.name = name->name;
 	args.namelen = name->len;
-	args.value = value;
+	args.value = (uchar_t *)value;
 	args.valuelen = *valuelenp;
 	args.flags = flags;
 	args.hashval = xfs_da_hashname(args.name, args.namelen);
@@ -233,7 +233,7 @@ xfs_attr_set_int(xfs_inode_t *dp, struct xfs_name *name,
 	memset((char *)&args, 0, sizeof(args));
 	args.name = name->name;
 	args.namelen = name->len;
-	args.value = value;
+	args.value = (uchar_t *)value;
 	args.valuelen = valuelen;
 	args.flags = flags;
 	args.hashval = xfs_da_hashname(args.name, args.namelen);
@@ -1545,7 +1545,7 @@ xfs_attr_rmtval_get(xfs_da_args_t *args)
 	ASSERT(!(args->flags & ATTR_KERNOVAL));
 
 	mp = args->dp->i_mount;
-	dst = args->value;
+	dst = (xfs_caddr_t)args->value;
 	valuelen = args->valuelen;
 	lblkno = args->rmtblkno;
 	while (valuelen > 0) {
@@ -1601,7 +1601,7 @@ xfs_attr_rmtval_set(xfs_da_args_t *args)
 
 	dp = args->dp;
 	mp = dp->i_mount;
-	src = args->value;
+	src = (xfs_caddr_t)args->value;
 
 	/*
 	 * Find a "hole" in the attribute address space large enough for
libxfs/xfs_attr_leaf.c +5 -5
index f8f926f..d535752 100644
@@ -476,11 +476,11 @@ xfs_attr_shortform_to_leaf(xfs_da_args_t *args)
 
 	sfe = &sf->list[0];
 	for (i = 0; i < sf->hdr.count; i++) {
-		nargs.name = (char *)sfe->nameval;
+		nargs.name = (uchar_t *)sfe->nameval;
 		nargs.namelen = sfe->namelen;
-		nargs.value = (char *)&sfe->nameval[nargs.namelen];
+		nargs.value = (uchar_t *)&sfe->nameval[nargs.namelen];
 		nargs.valuelen = sfe->valuelen;
-		nargs.hashval = xfs_da_hashname((char *)sfe->nameval,
+		nargs.hashval = xfs_da_hashname((uchar_t *)sfe->nameval,
 						sfe->namelen);
 		nargs.flags = XFS_ATTR_NSP_ONDISK_TO_ARGS(sfe->flags);
 		error = xfs_attr_leaf_lookup_int(bp, &nargs); /* set a->index */
@@ -610,9 +610,9 @@ xfs_attr_leaf_to_shortform(xfs_dabuf_t *bp, xfs_da_args_t *args, int forkoff)
 			continue;
 		ASSERT(entry->flags & XFS_ATTR_LOCAL);
 		name_loc = XFS_ATTR_LEAF_NAME_LOCAL(leaf, i);
-		nargs.name = (char *)name_loc->nameval;
+		nargs.name = (uchar_t *)name_loc->nameval;
 		nargs.namelen = name_loc->namelen;
-		nargs.value = (char *)&name_loc->nameval[nargs.namelen];
+		nargs.value = (uchar_t *)&name_loc->nameval[nargs.namelen];
 		nargs.valuelen = be16_to_cpu(name_loc->valuelen);
 		nargs.hashval = be32_to_cpu(entry->hashval);
 		nargs.flags = XFS_ATTR_NSP_ONDISK_TO_ARGS(entry->flags);
libxfs/xfs_dir2.c +1 -1
index 71134f8..2d2abd1 100644
@@ -18,7 +18,7 @@
 
 #include <xfs.h>
 
-struct xfs_name xfs_name_dotdot = {"..", 2};
+struct xfs_name xfs_name_dotdot = {(uchar_t *)"..", 2};
 
 extern const struct xfs_nameops xfs_default_nameops;
 
libxfs/xfs_dir2_block.c +4 -4
index d197b0b..d286000 100644
@@ -36,8 +36,8 @@ static xfs_dahash_t xfs_dir_hash_dot, xfs_dir_hash_dotdot;
 void
 xfs_dir_startup(void)
 {
-	xfs_dir_hash_dot = xfs_da_hashname(".", 1);
-	xfs_dir_hash_dotdot = xfs_da_hashname("..", 2);
+	xfs_dir_hash_dot = xfs_da_hashname((uchar_t *)".", 1);
+	xfs_dir_hash_dotdot = xfs_da_hashname((uchar_t *)"..", 2);
 }
 
 /*
@@ -489,7 +489,7 @@ xfs_dir2_block_lookup(
 	 * Fill in inode number, CI name if appropriate, release the block.
 	 */
 	args->inumber = be64_to_cpu(dep->inumber);
-	error = xfs_dir_cilookup_result(args, dep->name, dep->namelen);
+	error = xfs_dir_cilookup_result(args, (char *)dep->name, dep->namelen);
 	xfs_da_brelse(args->trans, bp);
 	return XFS_ERROR(error);
 }
@@ -576,7 +576,7 @@ xfs_dir2_block_lookup_int(
 		 * and buffer. If it's the first case-insensitive match, store
 		 * the index and buffer and continue looking for an exact match.
 		 */
-		cmp = mp->m_dirnameops->compname(args, dep->name, dep->namelen);
+		cmp = mp->m_dirnameops->compname(args, (char *)dep->name, dep->namelen);
 		if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
 			args->cmpresult = cmp;
 			*bpp = bp;
libxfs/xfs_dir2_leaf.c +2 -2
index 9a1aace..356b541 100644
@@ -906,7 +906,7 @@ xfs_dir2_leaf_lookup(
 	 * Return the found inode number & CI name if appropriate
 	 */
 	args->inumber = be64_to_cpu(dep->inumber);
-	error = xfs_dir_cilookup_result(args, dep->name, dep->namelen);
+	error = xfs_dir_cilookup_result(args, (char *)dep->name, dep->namelen);
 	xfs_da_brelse(tp, dbp);
 	xfs_da_brelse(tp, lbp);
 	return XFS_ERROR(error);
@@ -1000,7 +1000,7 @@ xfs_dir2_leaf_lookup_int(
 		 * and buffer. If it's the first case-insensitive match, store
 		 * the index and buffer and continue looking for an exact match.
 		 */
-		cmp = mp->m_dirnameops->compname(args, dep->name, dep->namelen);
+		cmp = mp->m_dirnameops->compname(args, (char *)dep->name, dep->namelen);
 		if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
 			args->cmpresult = cmp;
 			*indexp = index;
libxfs/xfs_dir2_node.c +2 -2
index db88adc..6a92f13 100644
@@ -612,7 +612,7 @@ xfs_dir2_leafn_lookup_for_entry(
 		 * EEXIST immediately. If it's the first case-insensitive
 		 * match, store the block & inode number and continue looking.
 		 */
-		cmp = mp->m_dirnameops->compname(args, dep->name, dep->namelen);
+		cmp = mp->m_dirnameops->compname(args, (char *)dep->name, dep->namelen);
 		if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
 			/* If there is a CI match block, drop it */
 			if (args->cmpresult != XFS_CMP_DIFFERENT &&
@@ -1821,7 +1821,7 @@ xfs_dir2_node_lookup(
 
 		dep = (xfs_dir2_data_entry_t *)((char *)state->extrablk.bp->
 						data + state->extrablk.index);
-		rval = xfs_dir_cilookup_result(args, dep->name, dep->namelen);
+		rval = xfs_dir_cilookup_result(args, (char *)dep->name, dep->namelen);
 	}
 	/*
 	 * Release the btree blocks and leaf block.
libxfs/xfs_dir2_sf.c +7 -6
index 77f30de..06e076d 100644
@@ -732,8 +732,8 @@ xfs_dir2_sf_lookup(
 		 * number. If it's the first case-insensitive match, store the
 		 * inode number and continue looking for an exact match.
 		 */
-		cmp = dp->i_mount->m_dirnameops->compname(args, sfep->name,
-								sfep->namelen);
+		cmp = dp->i_mount->m_dirnameops->compname(args,
+					(char *)sfep->name, sfep->namelen);
 		if (cmp != XFS_CMP_DIFFERENT && cmp != args->cmpresult) {
 			args->cmpresult = cmp;
 			args->inumber = xfs_dir2_sf_get_inumber(sfp,
@@ -751,7 +751,8 @@ xfs_dir2_sf_lookup(
 	if (!ci_sfep)
 		return XFS_ERROR(ENOENT);
 	/* otherwise process the CI match as required by the caller */
-	error = xfs_dir_cilookup_result(args, ci_sfep->name, ci_sfep->namelen);
+	error = xfs_dir_cilookup_result(args, (char *)ci_sfep->name,
+						ci_sfep->namelen);
 	return XFS_ERROR(error);
 }
 
@@ -793,7 +794,7 @@ xfs_dir2_sf_removename(
 	 */
 	for (i = 0, sfep = xfs_dir2_sf_firstentry(sfp); i < sfp->hdr.count;
 				i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
-		if (xfs_da_compname(args, sfep->name, sfep->namelen) ==
+		if (xfs_da_compname(args, (char *)sfep->name, sfep->namelen) ==
 								XFS_CMP_EXACT) {
 			ASSERT(xfs_dir2_sf_get_inumber(sfp,
 						xfs_dir2_sf_inumberp(sfep)) ==
@@ -928,8 +929,8 @@ xfs_dir2_sf_replace(
 		for (i = 0, sfep = xfs_dir2_sf_firstentry(sfp);
 				i < sfp->hdr.count;
 				i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
-			if (xfs_da_compname(args, sfep->name, sfep->namelen) ==
-								XFS_CMP_EXACT) {
+			if (xfs_da_compname(args, (char *)sfep->name,
+					sfep->namelen) == XFS_CMP_EXACT) {
 #if XFS_BIG_INUMS || defined(DEBUG)
 				ino = xfs_dir2_sf_get_inumber(sfp,
 					xfs_dir2_sf_inumberp(sfep));
Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#2 lore raw

Re: [PATCH 1/9] xfsprogs: fix sign warnings in libxfs

Quoted 4 lines · Dave Chinner

The directory naming is a mix of char and uchar_t and the compiler warns wheter they cross over. Cast the pointers according to the destination requirements as it doesn't seem to matter to avoid the warnings.

I think this makes sense, but it should really go into the kernel code first - we currently paper over the warnings there by using -funsigned-char for all XFS code which we might be able to remove with those changes.

include/xfs_types.h typedef enum { · line 165

Quoted 5 lines

struct xfs_name {

include/xfs_types.h +1 -1
-	const char	*name;
+	const uchar_t	*name;
 	int		len;

Maye use on of the standard types (unsigned char / u8 / uint8_t) instead of the weird xfs-specific uchar_t, which in fact is already gone in the kernel?

Quoted 10 lines
libxfs/xfs_attr.c +1 -1
@@ -1545,7 +1545,7 @@ xfs_attr_rmtval_get(xfs_da_args_t *args)
 	ASSERT(!(args->flags & ATTR_KERNOVAL));
 
 	mp = args->dp->i_mount;
-	dst = args->value;
+	dst = (xfs_caddr_t)args->value;
 	valuelen = args->valuelen;
 	lblkno = args->rmtblkno;
 	while (valuelen > 0) {

libxfs/xfs_attr.c xfs_attr_rmtval_set(xfs_da_args_t *args) · line 1604

Quoted 5 lines
libxfs/xfs_attr.c +1 -1
@@ -1601,7 +1601,7 @@ xfs_attr_rmtval_set(xfs_da_args_t *args)
 
 	dp = args->dp;
 	mp = dp->i_mount;
-	src = args->value;
+	src = (xfs_caddr_t)args->value;

If we make xfs_buf_iomove take a void * we can just pass on the unsignedness in those two places.

Quoted 7 lines
libxfs/xfs_attr_leaf.c +0 -0
index f8f926f..d535752 100644
@@ -476,11 +476,11 @@ xfs_attr_shortform_to_leaf(xfs_da_args_t *args)
 
 	sfe = &sf->list[0];

libxfs/xfs_attr_leaf.c xfs_attr_shortform_to_leaf(xfs_da_args_t *args) · line 484

Quoted 10 lines
libxfs/xfs_attr_leaf.c +3 -3
 	for (i = 0; i < sf->hdr.count; i++) {
-		nargs.name = (char *)sfe->nameval;
+		nargs.name = (uchar_t *)sfe->nameval;
 		nargs.namelen = sfe->namelen;
-		nargs.value = (char *)&sfe->nameval[nargs.namelen];
+		nargs.value = (uchar_t *)&sfe->nameval[nargs.namelen];
 		nargs.valuelen = sfe->valuelen;
-		nargs.hashval = xfs_da_hashname((char *)sfe->nameval,
+		nargs.hashval = xfs_da_hashname((uchar_t *)sfe->nameval,
 						sfe->namelen);

Both the leaf and sf nameval fields already are __u8/__uint8_t aka unsigned char, so I can't see why can't see why any casts are needed here and in the later hunks dealing with namevals.

libxfs/xfs_attr_leaf.c xfs_dir2_block_lookup( · line 492

Quoted 6 lines
libxfs/xfs_attr_leaf.c +1 -1
@@ -489,7 +489,7 @@ xfs_dir2_block_lookup(
 	 * Fill in inode number, CI name if appropriate, release the block.
 	 */
 	args->inumber = be64_to_cpu(dep->inumber);
-	error = xfs_dir_cilookup_result(args, dep->name, dep->namelen);
+	error = xfs_dir_cilookup_result(args, (char *)dep->name, dep->namelen);

xfs_dir_cilookup_result always gets unsigned char names now, so I'd move the conversion into it.

libxfs/xfs_attr_leaf.c xfs_dir2_block_lookup_int( · line 579

Quoted 9 lines
libxfs/xfs_attr_leaf.c +1 -1
 	xfs_da_brelse(args->trans, bp);
 	return XFS_ERROR(error);
 }
@@ -576,7 +576,7 @@ xfs_dir2_block_lookup_int(
 		 * and buffer. If it's the first case-insensitive match, store
 		 * the index and buffer and continue looking for an exact match.
 		 */
-		cmp = mp->m_dirnameops->compname(args, dep->name, dep->namelen);
+		cmp = mp->m_dirnameops->compname(args, (char *)dep->name, dep->namelen);

Also here, ->compname appears to always get unsigned names.

libxfs/xfs_attr_leaf.c xfs_dir2_sf_replace( · line 933

Quoted 13 lines
libxfs/xfs_attr_leaf.c +1 -1
-		if (xfs_da_compname(args, sfep->name, sfep->namelen) ==
+		if (xfs_da_compname(args, (char *)sfep->name, sfep->namelen) ==
 								XFS_CMP_EXACT) {
 			ASSERT(xfs_dir2_sf_get_inumber(sfp,
libxfs/xfs_attr_leaf.c +2 -2
 						xfs_dir2_sf_inumberp(sfep)) ==
@@ -928,8 +929,8 @@ xfs_dir2_sf_replace(
 		for (i = 0, sfep = xfs_dir2_sf_firstentry(sfp);
 				i < sfp->hdr.count;
 				i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
-			if (xfs_da_compname(args, sfep->name, sfep->namelen) ==
-								XFS_CMP_EXACT) {
+			if (xfs_da_compname(args, (char *)sfep->name,
+					sfep->namelen) == XFS_CMP_EXACT) {

Same seems to apply for direct calls to xfs_da_compname.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#3 lore raw

PATCH 2/9 xfsprogs: Fix miscellaneous warnings in libxfs

In reply to Dave Chinner

These are questionable fixes but they do shut up the warnings with as little change as possible. The first couple are questionable because they are a result of the compiler (gcc 4.3.4) trying to be smart but failing.

The uninitialised variable warnings in xfs_bmapi() is spurious, as certain fields are unused in certain situations but gcc can't work that out. It is easily suppressed by initialising the structure to zero.

xfs_mod_sb() is dumping out an array subscript bound warning that is spurious. The code is correct and will assert fail on out of bounds indexes. A simple reorder of the first and last offset calculations makes the warning go away (a good sign of compiler fail if ever there was one).

Changing xfs_alloc_mark_busy() from a macro to a static inline definition is needed for the compiler to see that variables passed to that function are actually used.

---
 libxfs/xfs.h       |    5 +++--
 libxfs/xfs_bmap.c  |    2 +-
 libxfs/xfs_mount.c |    9 ++++-----
 3 files changed, 8 insertions(+), 8 deletions(-)
libxfs/xfs.h +3 -2
index 8e94dad..abe8700 100644
@@ -288,9 +288,10 @@ roundup_pow_of_two(uint v)
 #define xfs_ilock(ip,mode)				((void) 0)
 #define xfs_iunlock(ip,mode)				((void) 0)
 
-/* space allocation */
+/* space allocation - use static inlines to avoid unused var warnings */
 #define xfs_alloc_search_busy(tp,ag,b,len)	((void) 0)
-#define xfs_alloc_mark_busy(tp,ag,b,len)	((void) 0)
+static inline void xfs_alloc_mark_busy(xfs_trans_t *tp,
+	xfs_agnumber_t agno, xfs_agblock_t bno, xfs_extlen_t len) {}
 #define xfs_rotorstep				1
 #define xfs_bmap_rtalloc(a)			(ENOSYS)
 #define xfs_rtpick_extent(mp,tp,len,p)		(ENOSYS)
libxfs/xfs_bmap.c +1 -1
index cf1123a..4cc6431 100644
@@ -4274,7 +4274,7 @@ xfs_bmapi(
 	xfs_fsblock_t	abno;		/* allocated block number */
 	xfs_extlen_t	alen;		/* allocated extent length */
 	xfs_fileoff_t	aoff;		/* allocated file offset */
-	xfs_bmalloca_t	bma;		/* args for xfs_bmap_alloc */
+	xfs_bmalloca_t	bma = { 0 };	/* args for xfs_bmap_alloc */
 	xfs_btree_cur_t	*cur;		/* bmap btree cursor */
 	xfs_fileoff_t	end;		/* end of mapped file region */
 	int		eof;		/* we've hit the end of extents */
libxfs/xfs_mount.c +4 -5
index 02bff42..662c8d2 100644
@@ -400,15 +400,14 @@ xfs_mod_sb(xfs_trans_t *tp, __int64_t fields)
 
 	xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb, fields);
 
-	/* find modified range */
+	/* find modified range - get last first to avoid spurious warning */
+	f = (xfs_sb_field_t)xfs_highbit64((__uint64_t)fields);
+	ASSERT((1LL << f) & XFS_SB_MOD_BITS);
+	last = xfs_sb_info[f + 1].offset - 1;
 
 	f = (xfs_sb_field_t)xfs_lowbit64((__uint64_t)fields);
 	ASSERT((1LL << f) & XFS_SB_MOD_BITS);
 	first = xfs_sb_info[f].offset;
 
-	f = (xfs_sb_field_t)xfs_highbit64((__uint64_t)fields);
-	ASSERT((1LL << f) & XFS_SB_MOD_BITS);
-	last = xfs_sb_info[f + 1].offset - 1;
-
 	xfs_trans_log_buf(tp, bp, first, last);
 }
Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#4 lore raw

Re: [PATCH 2/9] xfsprogs: Fix miscellaneous warnings in libxfs

libxfs/xfs.h roundup_pow_of_two(uint v) · line 297

Quoted 9 lines
libxfs/xfs.h +3 -2
-/* space allocation */
+/* space allocation - use static inlines to avoid unused var warnings */
 #define xfs_alloc_search_busy(tp,ag,b,len)	((void) 0)
-#define xfs_alloc_mark_busy(tp,ag,b,len)	((void) 0)
+static inline void xfs_alloc_mark_busy(xfs_trans_t *tp,
+	xfs_agnumber_t agno, xfs_agblock_t bno, xfs_extlen_t len) {}
 #define xfs_rotorstep				1
 #define xfs_bmap_rtalloc(a)			(ENOSYS)
 #define xfs_rtpick_extent(mp,tp,len,p)		(ENOSYS)

Maybe just convert all these to inlines so that we make sure all arguments are evaluated?

libxfs/xfs_bmap.c xfs_bmapi( · line 4280

Quoted 13 lines
libxfs/xfs_bmap.c +1 -1
index cf1123a..4cc6431 100644
@@ -4274,7 +4274,7 @@ xfs_bmapi(
 	xfs_fsblock_t	abno;		/* allocated block number */
 	xfs_extlen_t	alen;		/* allocated extent length */
 	xfs_fileoff_t	aoff;		/* allocated file offset */
-	xfs_bmalloca_t	bma;		/* args for xfs_bmap_alloc */
+	xfs_bmalloca_t	bma = { 0 };	/* args for xfs_bmap_alloc */
 	xfs_btree_cur_t	*cur;		/* bmap btree cursor */
 	xfs_fileoff_t	end;		/* end of mapped file region */
 	int		eof;		/* we've hit the end of extents */

Not nice but okay..

Quoted 12 lines
libxfs/xfs_mount.c +3 -1
index 02bff42..662c8d2 100644
@@ -400,15 +400,14 @@ xfs_mod_sb(xfs_trans_t *tp, __int64_t fields)
 
 	xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb, fields);
 
-	/* find modified range */
+	/* find modified range - get last first to avoid spurious warning */
+	f = (xfs_sb_field_t)xfs_highbit64((__uint64_t)fields);
+	ASSERT((1LL << f) & XFS_SB_MOD_BITS);

libxfs/xfs_mount.c xfs_mod_sb(xfs_trans_t *tp, __int64_t fields) · line 412

Quoted 10 lines
libxfs/xfs_mount.c +1 -4
+	last = xfs_sb_info[f + 1].offset - 1;
 
 	f = (xfs_sb_field_t)xfs_lowbit64((__uint64_t)fields);
 	ASSERT((1LL << f) & XFS_SB_MOD_BITS);
 	first = xfs_sb_info[f].offset;
 
-	f = (xfs_sb_field_t)xfs_highbit64((__uint64_t)fields);
-	ASSERT((1LL << f) & XFS_SB_MOD_BITS);
-	last = xfs_sb_info[f + 1].offset - 1;
-

I don't see how this can shut up a warning, but as it doesn't cause harm I'm fine with it. But this and the above one really need to go in via the kernel tree first.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#5 lore raw

PATCH 3/9 xfsprogs: fix missing error check in xfs_rtfree_range in libxfs

In reply to Dave Chinner

When xfs_rtfind_forw() returns an error, the block is returned uninitialised. xfs_rtfree_range() is not checking the error return, so could be using an uninitialised block number for modifying bitmap summary info.

---
 libxfs/xfs_rtalloc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
libxfs/xfs_rtalloc.c +2 -0
index 6c3da52..4fbdaa9 100644
@@ -466,6 +466,8 @@ xfs_rtfree_range(
 	 */
 	error = xfs_rtfind_forw(mp, tp, end, mp->m_sb.sb_rextents - 1,
 		&postblock);
+	if (error)
+		return error;
 	/*
 	 * If there are blocks not being freed at the front of the
 	 * old extent, add summary data for them to be allocated.
Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Reviewed-by on v1 3/9
#6 lore raw

Re: [PATCH 3/9] xfsprogs: fix missing error check in xfs_rtfree_range in libxfs

Quoted 4 lines · Dave Chinner

When xfs_rtfind_forw() returns an error, the block is returned uninitialised. xfs_rtfree_range() is not checking the error return, so could be using an uninitialised block number for modifying bitmap summary info.

Looks good, (actually a merge of the kernel side fix that just went in)

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#7 lore raw

PATCH 4/9 xfsprogs: fix warning in adfs superblock probe

In reply to Dave Chinner

The probe gets an array subscript warning because gcc is not smart enough to realise that a structure made up of multiple byte arrays in it can be referenced as a flat buffer and it is valid to access bytes beyond the first array in the structure....

Fix it by passing the adfs superblock in and using the internal checksum array to get the checksum value.

---
 libdisk/fstype.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
libdisk/fstype.c +5 -5
index f84b4e4..548f297 100644
@@ -142,16 +142,16 @@ may_be_swap(const char *s) {
 
 /* rather weak necessary condition */
 static int
-may_be_adfs(const char *s) {
+may_be_adfs(const struct adfs_super_block *sb) {
 	char *p;
 	int sum;
 
-	p = (char *) s + 511;
+	p = (char *)sb->s_checksum;
 	sum = 0;
-	while(--p != s)
+	while(--p != (char *)sb)
 		sum = (sum >> 8) + (sum & 0xff) + *p;
 
-	return (sum == p[511]);
+	return (sum & 0xff) == sb->s_checksum[0];
 }
 
 static int is_reiserfs_magic_string (struct reiserfs_super_block * rs)
@@ -304,7 +304,7 @@ fstype(const char *device) {
              goto io_error;
 
 	/* only a weak test */
-        if (may_be_adfs((char *) &adfssb)
+        if (may_be_adfs(&adfssb)
             && (adfsblksize(adfssb) >= 8 &&
                 adfsblksize(adfssb) <= 10))
              type = "adfs";
Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Reviewed-by on v1 4/9
#8 lore raw

Re: [PATCH 4/9] xfsprogs: fix warning in adfs superblock probe

Quoted 7 lines · Dave Chinner

The probe gets an array subscript warning because gcc is not smart enough to realise that a structure made up of multiple byte arrays in it can be referenced as a flat buffer and it is valid to access bytes beyond the first array in the structure....

Fix it by passing the adfs superblock in and using the internal checksum array to get the checksum value.

The check still seems horribly ugly, but hey given that this is code superceeded by blkid it should be good enough,

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#9 lore raw

PATCH 5/9 xfsprogs: fix some trivial warnings in xfs_db

In reply to Dave Chinner

Three warnings, none harmful - one dir2 name sign warning and two cases where gcc can't work out if a variable is initialised correctly in a child function or not.

---
 db/check.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
db/check.c +3 -3
index 7620d9c..f94a16b 100644
@@ -2317,7 +2317,7 @@ process_data_dir_v2(
 		tag_err += be16_to_cpu(*tagp) != (char *)dep - (char *)data;
 		addr = xfs_dir2_db_off_to_dataptr(mp, db,
 			(char *)dep - (char *)data);
-		xname.name = (char *)dep->name;
+		xname.name = (uchar_t *)dep->name;
 		xname.len = dep->namelen;
 		dir_hash_add(mp->m_dirnameops->hashname(&xname), addr);
 		ptr += xfs_dir2_data_entsize(dep->namelen);
@@ -3059,7 +3059,7 @@ process_leaf_node_dir_v2(
 	xfs_ino_t		lino;
 	int			nex;
 	xfs_ino_t		parent;
-	int			t;
+	int			t = 0;
 	int			v;
 	int			v2;
 	int			x;
@@ -3403,7 +3403,7 @@ process_quota(
 	xfs_fileoff_t	qbno;
 	char		*s = NULL;
 	int		scicb;
-	int		t;
+	int		t = 0;
 
 	switch (qtype) {
 	case IS_USER_QUOTA:
Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Reviewed-by on v1 5/9
#10 lore raw

Re: [PATCH 5/9] xfsprogs: fix some trivial warnings in xfs_db

Quoted 3 lines · Dave Chinner

Three warnings, none harmful - one dir2 name sign warning and two cases where gcc can't work out if a variable is initialised correctly in a child function or not.

Might be worth to throw all the dabtree sign things in with the mign signed issue patch?

Anyway, the changes look good,

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#11 lore raw

PATCH 6/9 xfsprogs: fix trivial warnings in xfs_fsr

In reply to Dave Chinner

GCC complains about assignments between (unsigned long long *) and (__u64 *) pointers. Just add a cast to shut it up.

---
 fsr/xfs_fsr.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
fsr/xfs_fsr.c +2 -2
index 21f08c3..1f933c7 100644
@@ -133,7 +133,7 @@ xfs_bulkstat_single(int fd, xfs_ino_t *lastip, xfs_bstat_t *ubuffer)
 {
     xfs_fsop_bulkreq_t  bulkreq;
 
-    bulkreq.lastip = lastip;
+    bulkreq.lastip = (__u64 *)lastip;
     bulkreq.icount = 1;
     bulkreq.ubuffer = ubuffer;
     bulkreq.ocount = NULL;
@@ -146,7 +146,7 @@ xfs_bulkstat(int fd, xfs_ino_t *lastip, int icount,
 {
     xfs_fsop_bulkreq_t  bulkreq;
 
-    bulkreq.lastip = lastip;
+    bulkreq.lastip = (__u64 *)lastip;
     bulkreq.icount = icount;
     bulkreq.ubuffer = ubuffer;
     bulkreq.ocount = ocount;
Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Reviewed-by on v1 6/9
#12 lore raw

Re: [PATCH 6/9] xfsprogs: fix trivial warnings in xfs_fsr

Quoted 4 lines · Dave Chinner

GCC complains about assignments between (unsigned long long *) and (__u64 *) pointers. Just add a cast to shut it up.

Looks good,

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#13 lore raw

PATCH 7/9 xfsprogs: fix print format warnings in xfs_io

In reply to Dave Chinner
---
 io/madvise.c |    2 +-
 io/mincore.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
io/madvise.c +1 -1
index cd16a4c..3235402 100644
@@ -96,7 +96,7 @@ madvise_f(
 			return 0;
 		} else if (llength > (size_t)llength) {
 			printf(_("length argument too large -- %lld\n"),
-				llength);
+				(long long)llength);
 			return 0;
 		} else
 			length = (size_t)llength;
io/mincore.c +1 -1
index d534540..60625ef 100644
@@ -56,7 +56,7 @@ mincore_f(
 			return 0;
 		} else if (llength > (size_t)llength) {
 			printf(_("length argument too large -- %lld\n"),
-				llength);
+				(long long)llength);
 			return 0;
 		} else
 			length = (size_t)llength;
Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#15 lore raw

PATCH 8/9 xfsprogs: fix sign warning in mkfs directory code

In reply to Dave Chinner
---
 mkfs/proto.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
mkfs/proto.c +1 -1
index 8fc8c4c..3723685 100644
@@ -432,7 +432,7 @@ parseproto(
 	mode |= val;
 	creds.cr_uid = (int)getnum(pp);
 	creds.cr_gid = (int)getnum(pp);
-	xname.name = name;
+	xname.name = (uchar_t *)name;
 	xname.len = name ? strlen(name) : 0;
 	tp = libxfs_trans_alloc(mp, 0);
 	flags = XFS_ILOG_CORE;
Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#17 lore raw

PATCH 9/9 xfsprogs: fix build warnings in repair

In reply to Dave Chinner

Rewrite the loop in btree_get_prev() so that the compiler can see that it returns if the cur->index is zero so it doesn't complain about possible array bound underflows when getting the key out of the buffer.

Fix the directory name sign warnings by casting to (uchar_t *) appropriately.

---
 repair/btree.c  |    9 +++++----
 repair/phase6.c |   10 +++++-----
 2 files changed, 10 insertions(+), 9 deletions(-)
repair/btree.c +5 -4
index 3141388..b53b4e4 100644
@@ -194,12 +194,13 @@ btree_get_prev(
 	}
 
 	/* else need to go up and back down the tree to find the previous */
-
-	while (cur->index == 0) {
-		if (++level == root->height)
-			return NULL;
+	while (++level < root->height) {
+		if (cur->index)
+			break;
 		cur++;
 	}
+	if (cur->index == 0)
+		return NULL;
 
 	/* the key is in the current level */
 	if (key)
repair/phase6.c +5 -5
index d056063..884ad62 100644
@@ -35,7 +35,7 @@ static struct cred		zerocr;
 static struct fsxattr 		zerofsx;
 static xfs_ino_t		orphanage_ino;
 
-static struct xfs_name 		xfs_name_dot = {".", 1};
+static struct xfs_name 		xfs_name_dot = {(uchar_t *)".", 1};
 
 /*
  * Data structures used to keep track of directories where the ".."
@@ -145,7 +145,7 @@ dir_hash_add(
 
 	ASSERT(!hashtab->names_duped);
 
-	xname.name = name;
+	xname.name = (uchar_t *)name;
 	xname.len = namelen;
 
 	junk = name[0] == '/';
@@ -355,7 +355,7 @@ dir_hash_dup_names(dir_hash_tab_t *hashtab)
 	for (p = hashtab->first; p; p = p->nextbyorder) {
 		name = malloc(p->name.len);
 		memcpy(name, p->name.name, p->name.len);
-		p->name.name = name;
+		p->name.name = (uchar_t *)name;
 	}
 	hashtab->names_duped = 1;
 }
@@ -843,7 +843,7 @@ mk_orphanage(xfs_mount_t *mp)
 		do_error(_("%d - couldn't iget root inode to obtain %s\n"),
 			i, ORPHANAGE);
 
-	xname.name = ORPHANAGE;
+	xname.name = (uchar_t *)ORPHANAGE;
 	xname.len = strlen(ORPHANAGE);
 	if (libxfs_dir_lookup(NULL, pip, &xname, &ino, NULL) == 0)
 		return ino;
@@ -944,7 +944,7 @@ mv_orphanage(
 
 	ASSERT(xfs_sb_version_hasdirv2(&mp->m_sb));
 
-	xname.name = fname;
+	xname.name = (uchar_t *)fname;
 	xname.len = snprintf(fname, sizeof(fname), "%llu",
 				(unsigned long long)ino);
 
Signature · 6 lines
1.6.5

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#18 lore raw

Re: [PATCH 9/9] xfsprogs: fix build warnings in repair

Quoted 23 lines · Dave Chinner

Rewrite the loop in btree_get_prev() so that the compiler can see that it returns if the cur->index is zero so it doesn't complain about possible array bound underflows when getting the key out of the buffer.

Fix the directory name sign warnings by casting to (uchar_t *) appropriately.

---
 repair/btree.c  |    9 +++++----
 repair/phase6.c |   10 +++++-----
 2 files changed, 10 insertions(+), 9 deletions(-)
repair/btree.c +0 -1
index 3141388..b53b4e4 100644
@@ -194,12 +194,13 @@ btree_get_prev(
 	}
 
 	/* else need to go up and back down the tree to find the previous */
-

repair/btree.c btree_get_prev( · line 203

Quoted 10 lines
repair/btree.c +5 -3
-	while (cur->index == 0) {
-		if (++level == root->height)
-			return NULL;
+	while (++level < root->height) {
+		if (cur->index)
+			break;
 		cur++;
 	}
+	if (cur->index == 0)
+		return NULL;

This is broken - causes a segv. i have a fixed patch, which I'll send out once I've got xfstests running again on my test box (using dash as /bin/sh breaks *lots* of stuff in xfstests).

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs