[PATCH 0/7] xfsprogs: Fix build warnings V3

[PATCH 0/7] xfsprogs: Fix build warnings V3

[Dave Chinner]

This is mostly the same series as posted last time, but I've dropped
the sign warning changes in libxfs until the kernel code is fixed.

The only other change is patch 7/7 - this version doesn't fall off the
top of the btree and SEGV during xfsqa runs.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/7] xfsprogs: fix missing error check in xfs_rtfree_range in libxfs

[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.

Signed-off-by: Dave Chinner <david@fromorbit.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 libxfs/xfs_rtalloc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/libxfs/xfs_rtalloc.c b/libxfs/xfs_rtalloc.c
index 6c3da52..4fbdaa9 100644
--- a/libxfs/xfs_rtalloc.c
+++ b/libxfs/xfs_rtalloc.c
@@ -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.
-- 
1.6.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 2/7] xfsprogs: fix warning in adfs superblock probe

[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.

Signed-off-by: Dave Chinner <david@fromorbit.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 libdisk/fstype.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/libdisk/fstype.c b/libdisk/fstype.c
index f84b4e4..548f297 100644
--- a/libdisk/fstype.c
+++ b/libdisk/fstype.c
@@ -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";
-- 
1.6.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 3/7] xfsprogs: fix some trivial warnings in xfs_db

[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.

Signed-off-by: Dave Chinner <david@fromorbit.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 db/check.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/db/check.c b/db/check.c
index 7620d9c..f94a16b 100644
--- a/db/check.c
+++ b/db/check.c
@@ -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:
-- 
1.6.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/7] xfsprogs: fix some trivial warnings in xfs_db

[Christoph Hellwig]

On Tue, Jan 19, 2010 at 11:36:49AM +1100, Dave Chinner wrote:
> -		xname.name = (char *)dep->name;
> +		xname.name = (uchar_t *)dep->name;

Aren't these both unsigned after the latest version of the signedness
patches are in?

Maybe just put the other one in for now.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 3/7] xfsprogs: fix some trivial warnings in xfs_db

[Christoph Hellwig]

On Tue, Jan 19, 2010 at 11:36:49AM +1100, Dave Chinner wrote:
> 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.
> 
> Signed-off-by: Dave Chinner <david@fromorbit.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  db/check.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/db/check.c b/db/check.c
> index 7620d9c..f94a16b 100644
> --- a/db/check.c
> +++ b/db/check.c
> @@ -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;

Aren't hese now both unsigned after the kernel changes are sycned over?

And btw, please also avoid uchar_t in xfsprogs, it will be gone with
next full libxfs resync.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 4/7] xfsprogs: fix trivial warnings in xfs_fsr

[Dave Chinner]

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

Signed-off-by: Dave Chinner <david@fromorbit.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 fsr/xfs_fsr.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 21f08c3..1f933c7 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -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;
-- 
1.6.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 5/7] xfsprogs: fix print format warnings in xfs_io

[Dave Chinner]

Signed-off-by: Dave Chinner <david@fromorbit.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 io/madvise.c |    2 +-
 io/mincore.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/io/madvise.c b/io/madvise.c
index cd16a4c..3235402 100644
--- a/io/madvise.c
+++ b/io/madvise.c
@@ -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;
diff --git a/io/mincore.c b/io/mincore.c
index d534540..60625ef 100644
--- a/io/mincore.c
+++ b/io/mincore.c
@@ -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;
-- 
1.6.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 6/7] xfsprogs: fix sign warning in mkfs directory code

[Dave Chinner]

Signed-off-by: Dave Chinner <david@fromorbit.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 mkfs/proto.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mkfs/proto.c b/mkfs/proto.c
index 8fc8c4c..3723685 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -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;
-- 
1.6.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 7/7] xfsprogs: fix build warnings in repair V2

[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. Version 2 fixes
a height overflow in the reworked loop.

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

Signed-off-by: Dave Chinner <david@fromorbit.com>
---
 repair/btree.c  |   12 +++++++-----
 repair/phase6.c |   10 +++++-----
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/repair/btree.c b/repair/btree.c
index 3141388..22ab4d9 100644
--- a/repair/btree.c
+++ b/repair/btree.c
@@ -194,12 +194,14 @@ 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;
+	do {
+		if (cur->index)
+			break;
 		cur++;
-	}
+	} while (++level < root->height);
+
+	if (level == root->height)
+		return NULL;
 
 	/* the key is in the current level */
 	if (key)
diff --git a/repair/phase6.c b/repair/phase6.c
index d056063..884ad62 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -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);
 
-- 
1.6.5

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs