[PATCH 00/18] xfsprogs: finer-grained sparse fixes

[PATCH 00/18] xfsprogs: finer-grained sparse fixes

[Eric Sandeen]

hch asked for finer-grained patches for some of the sparse warning
fixes, so here we go.

Thanks,
-Eric

[PATCH 01/18] xfsprogs: enable sparse checking with "make C=[12]"

[Eric Sandeen]

Enable "make C=1" or "make C=2" to do sparse checking.
Blatantly ripped off from djwong's patch in e2fsprogs to do the same.

This is a bit simpler than redefining CC for the whole build, which
requires extra commandline definitions and apparently is enough of a
barrier that nobody's doing sparse checking.

Note, this requires unreleased sparse after v0.5, which enables the
CHAR_BIT definition; otherwise it chokes.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 Makefile           | 14 ++++++++++++++
 include/buildrules |  2 ++
 2 files changed, 16 insertions(+)

diff --git a/Makefile b/Makefile
index d031a60..7f3e774 100644
--- a/Makefile
+++ b/Makefile
@@ -16,6 +16,20 @@ else
   Q = @
 endif
 
+CHECK=sparse
+CHECK_OPTS=-Wsparse-all -Wno-transparent-union -Wno-return-void -Wno-undef \
+	-Wno-non-pointer-null -D__linux__
+ifeq ("$(C)", "2")
+  CHECK_CMD=$(CHECK) $(CHECK_OPTS) -Wbitwise -D__CHECK_ENDIAN__
+else
+  ifeq ("$(C)", "1")
+    CHECK_CMD=$(CHECK) $(CHECK_OPTS)
+   else
+    CHECK_CMD=@true
+  endif
+endif
+export CHECK_CMD
+
 MAKEOPTS = --no-print-directory Q=$(Q)
 
 TOPDIR = .
diff --git a/include/buildrules b/include/buildrules
index c57fdfc..23fc866 100644
--- a/include/buildrules
+++ b/include/buildrules
@@ -54,10 +54,12 @@ $(LTLIBRARY) : $(SUBDIRS) $(LTOBJECTS)
 %.lo: %.c
 	@echo "    [CC]     $@"
 	$(Q)$(LTCOMPILE) -c $<
+	$(Q)$(CHECK_CMD) $(CFLAGS) $<
 else
 %.o: %.c
 	@echo "    [CC]     $@"
 	$(Q)$(CC) $(CFLAGS) -c $<
+	$(Q)$(CHECK_CMD) $(CFLAGS) $<
 
 endif
 
-- 
1.8.3.1

[PATCH 02/18] xfs_db: convert single-bit bitfields to bools

[Eric Sandeen]

Sparse doesn't like signed single-bit bitfields, and they may as well
just be booleans.

Fixes sparse warnings about this.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 db/check.c | 4 ++--
 db/io.h    | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/db/check.c b/db/check.c
index e0fd986..683e176 100644
--- a/db/check.c
+++ b/db/check.c
@@ -41,8 +41,8 @@ typedef struct inodata {
 	struct inodata	*next;
 	nlink_t		link_set;
 	nlink_t		link_add;
-	char		isdir:1;
-	char		isreflink:1;
+	bool		isdir;
+	bool		isreflink;
 	char		security;
 	char		ilist;
 	xfs_ino_t	ino;
diff --git a/db/io.h b/db/io.h
index 8d5720b..7618be4 100644
--- a/db/io.h
+++ b/db/io.h
@@ -26,10 +26,10 @@ typedef struct iocur {
 	const struct typ	*typ;	/* type of "data" */
 	bbmap_t			*bbmap;	/* map daddr if fragmented */
 	struct xfs_buf		*bp;	/* underlying buffer */
-	int			ino_crc_ok:1;
-	int			ino_buf:1;
-	int			dquot_buf:1;
-	int			need_crc:1;
+	bool			ino_crc_ok;
+	bool			ino_buf;
+	bool			dquot_buf;
+	bool			need_crc;
 } iocur_t;
 
 #define DB_RING_ADD 1                   /* add to ring on set_cur */
-- 
1.8.3.1

[PATCH 03/18] xfsprogs: minor endian annotation fixes

[Eric Sandeen]

No actual bugs, just quiet the sparse checker.

Fixes sparse warnings about this.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 copy/xfs_copy.c     | 2 +-
 db/crc.c            | 6 +++---
 db/write.c          | 4 ++--
 logprint/log_misc.c | 5 +++--
 repair/rmap.c       | 3 ++-
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/copy/xfs_copy.c b/copy/xfs_copy.c
index 64761b3..a6d6703 100644
--- a/copy/xfs_copy.c
+++ b/copy/xfs_copy.c
@@ -505,7 +505,7 @@ sb_update_uuid(
 	 */
 	if (xfs_sb_version_hascrc(sb) && !xfs_sb_version_hasmetauuid(sb) &&
 	    !uuid_equal(&tcarg->uuid, &sb->sb_uuid)) {
-		__be32 feat;
+		uint32_t feat;
 
 		feat = be32_to_cpu(ag_hdr->xfs_sb->sb_features_incompat);
 		feat |= XFS_SB_FEAT_INCOMPAT_META_UUID;
diff --git a/db/crc.c b/db/crc.c
index b6775bc..b6612a5 100644
--- a/db/crc.c
+++ b/db/crc.c
@@ -130,7 +130,7 @@ crc_f(
 		flist_t		*sfl;
 		int		bit_length;
 		int		parentoffset;
-		int		crc;
+		uint32_t	crc;
 
 		sfl = fl;
 		parentoffset = 0;
@@ -144,8 +144,8 @@ crc_f(
 		bit_length *= fcount(sfl->fld, iocur_top->data, parentoffset);
 		crc = getbitval(iocur_top->data, sfl->offset, bit_length,
 				BVUNSIGNED);
-		/* Off by one.. */
-		crc = cpu_to_be32(crc + 1);
+		/* Off by one, ignore endianness - we're just corrupting it. */
+		crc++;
 		setbitval(iocur_top->data, sfl->offset, bit_length, &crc);
 
 		/* Temporarily remove write verifier to write a bad CRC */
diff --git a/db/write.c b/db/write.c
index a48576b..e25d6ea 100644
--- a/db/write.c
+++ b/db/write.c
@@ -525,7 +525,7 @@ convert_arg(
 	char		*endp;
 	char		*rbuf;
 	char		*ostr;
-	__u64		*value;
+	__be64		*value;
 	__u64		val = 0;
 
 	if (bit_length <= 64)
@@ -535,7 +535,7 @@ convert_arg(
 
 	buf = xrealloc(buf, alloc_size);
 	memset(buf, 0, alloc_size);
-	value = (__u64 *)buf;
+	value = (__be64 *)buf;
 	rbuf = buf;
 
 	if (*arg == '\"') {
diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index e2889f0..e29366a 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -467,6 +467,7 @@ xlog_print_dir2_sf(
 	xfs_dir2_sf_hdr_t *sfp,
 	int		size)
 {
+	__be64		pino;	/* parent inode nr */
 	xfs_ino_t	ino;
 	int		count;
 	int		i;
@@ -481,8 +482,8 @@ xlog_print_dir2_sf(
 
 	printf(_("SHORTFORM DIRECTORY size %d count %d\n"),
 	       size, sfp->count);
-	memmove(&ino, &(sfp->parent), sizeof(ino));
-	printf(_(".. ino 0x%llx\n"), (unsigned long long) be64_to_cpu(ino));
+	memmove(&pino, &(sfp->parent), sizeof(pino));
+	printf(_(".. ino 0x%llx\n"), (unsigned long long) be64_to_cpu(pino));
 
 	count = sfp->count;
 	sfep = xfs_dir2_sf_firstentry(sfp);
diff --git a/repair/rmap.c b/repair/rmap.c
index 6de4a10..ebb5a3a 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -482,7 +482,8 @@ rmap_store_ag_btree_rec(
 	 */
 	agfl_bno = XFS_BUF_TO_AGFL_BNO(mp, agflbp);
 	b = agfl_bno + ag_rmaps[agno].ar_flcount;
-	while (*b != NULLAGBLOCK && b - agfl_bno < libxfs_agfl_size(mp)) {
+	while (*b != cpu_to_be32(NULLAGBLOCK) &&
+	       b - agfl_bno < libxfs_agfl_size(mp)) {
 		error = rmap_add_ag_rec(mp, agno, be32_to_cpu(*b), 1,
 				XFS_RMAP_OWN_AG);
 		if (error)
-- 
1.8.3.1

[PATCH 04/18] xfsprogs: avoid redefinition of NBBY

[Eric Sandeen]

Include sys/param.h for NBBY definition.  Do this before our local
guarded definition which is for platforms like android that don't have it,
see commit:

c9a90185 xfsprogs: define NBBY if not defined by the system header files

Fixes sparse warnings about this redefinition.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 include/platform_defs.h.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/platform_defs.h.in b/include/platform_defs.h.in
index c4f0e8b..d111ec6 100644
--- a/include/platform_defs.h.in
+++ b/include/platform_defs.h.in
@@ -19,6 +19,7 @@
 #include <pthread.h>
 #include <ctype.h>
 #include <sys/types.h>
+#include <sys/param.h>
 #include <limits.h>
 #include <stdbool.h>
 #include <libgen.h>
@@ -65,6 +66,7 @@ typedef unsigned short umode_t;
 #define max(a,b)	(((a)>(b))?(a):(b))
 #endif
 
+/* If param.h doesn't provide it, i.e. for Android */
 #ifndef NBBY
 #define NBBY 8
 #endif
-- 
1.8.3.1

[PATCH 05/18] xfsprogs: include headers for extern variables

[Eric Sandeen]

Include headers which export functions for their matching C files so that
they don't appear to be static to the sparse checker.

Fixes sparse warnings about this.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 db/crc.c        | 1 +
 db/fuzz.c       | 1 +
 db/hash.c       | 1 +
 db/logformat.c  | 1 +
 db/symlink.c    | 2 +-
 db/text.c       | 1 +
 libfrog/crc32.c | 1 +
 7 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/db/crc.c b/db/crc.c
index b6612a5..95161c6 100644
--- a/db/crc.c
+++ b/db/crc.c
@@ -17,6 +17,7 @@
 #include "output.h"
 #include "bit.h"
 #include "print.h"
+#include "crc.h"
 
 static int crc_f(int argc, char **argv);
 static void crc_help(void);
diff --git a/db/fuzz.c b/db/fuzz.c
index 5d5d54f..65157bd 100644
--- a/db/fuzz.c
+++ b/db/fuzz.c
@@ -21,6 +21,7 @@
 #include "print.h"
 #include "write.h"
 #include "malloc.h"
+#include "fuzz.h"
 
 static int	fuzz_f(int argc, char **argv);
 static void     fuzz_help(void);
diff --git a/db/hash.c b/db/hash.c
index bda3316..68c53e7 100644
--- a/db/hash.c
+++ b/db/hash.c
@@ -10,6 +10,7 @@
 #include "type.h"
 #include "io.h"
 #include "output.h"
+#include "hash.h"
 
 static int hash_f(int argc, char **argv);
 static void hash_help(void);
diff --git a/db/logformat.c b/db/logformat.c
index 42ce245..3374c29 100644
--- a/db/logformat.c
+++ b/db/logformat.c
@@ -9,6 +9,7 @@
 #include "init.h"
 #include "output.h"
 #include "libxlog.h"
+#include "logformat.h"
 
 #define MAX_LSUNIT	256 * 1024	/* max log buf. size */
 
diff --git a/db/symlink.c b/db/symlink.c
index ebf6637..752cae4 100644
--- a/db/symlink.c
+++ b/db/symlink.c
@@ -11,7 +11,7 @@
 #include "field.h"
 #include "bit.h"
 #include "init.h"
-
+#include "symlink.h"
 
 /*
  * XXX: no idea how to handle multiple contiguous block symlinks here.
diff --git a/db/text.c b/db/text.c
index 3a279d1..adf33a7 100644
--- a/db/text.c
+++ b/db/text.c
@@ -17,6 +17,7 @@
 #include "io.h"
 #include "output.h"
 #include "init.h"
+#include "text.h"
 
 static void     print_rawtext(void *data, int len);
 
diff --git a/libfrog/crc32.c b/libfrog/crc32.c
index 1d52f68..e3f3fd1 100644
--- a/libfrog/crc32.c
+++ b/libfrog/crc32.c
@@ -33,6 +33,7 @@
 #include "xfs.h"
 #include "xfs_arch.h"
 #include "crc32defs.h"
+#include "crc32c.h"
 
 /* types specifc to this file */
 typedef __u8	u8;
-- 
1.8.3.1

[PATCH 06/18] libxfs: add several zone extern declarations to libxfs_priv.h

[Eric Sandeen]

Several zones have extern declarations in kernelspace headers we don't
have in userspace.

Adding these to the libxfs_priv.h header silences sparse warnings about
whether these should be static vars.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/init.c        | 9 ---------
 libxfs/libxfs_priv.h | 7 +++++++
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/libxfs/init.c b/libxfs/init.c
index 10dcbf3..80fbe6a 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -363,15 +363,6 @@ done:
 static int
 manage_zones(int release)
 {
-	extern kmem_zone_t	*xfs_buf_zone;
-	extern kmem_zone_t	*xfs_ili_zone;
-	extern kmem_zone_t	*xfs_inode_zone;
-	extern kmem_zone_t	*xfs_ifork_zone;
-	extern kmem_zone_t	*xfs_buf_item_zone;
-	extern kmem_zone_t	*xfs_da_state_zone;
-	extern kmem_zone_t	*xfs_btree_cur_zone;
-	extern kmem_zone_t	*xfs_bmap_free_item_zone;
-	extern kmem_zone_t	*xfs_trans_zone;
 	extern void		xfs_dir_startup();
 
 	if (release) {	/* free zone allocation */
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 6df7864..b45d07e 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -55,6 +55,13 @@
 #include "xfs_fs.h"
 #include "crc32c.h"
 
+/* Zones used in libxfs allocations that aren't in shared header files */
+extern kmem_zone_t *xfs_buf_item_zone;
+extern kmem_zone_t *xfs_ili_zone;
+extern kmem_zone_t *xfs_buf_zone;
+extern kmem_zone_t *xfs_inode_zone;
+extern kmem_zone_t *xfs_trans_zone;
+
 /* CRC stuff, buffer API dependent on it */
 #define crc32c(c,p,l)	crc32c_le((c),(unsigned char const *)(p),(l))
 
-- 
1.8.3.1

[PATCH 07/18] libxfs: silence static warnings about platform_* functions

[Eric Sandeen]

Add all platform_* prototypes to init.h and include it in linux.c
to silence sparse warnings about static functions.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libfrog/linux.c | 1 +
 libxfs/init.h   | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/libfrog/linux.c b/libfrog/linux.c
index fc9f3ac..b6c2487 100644
--- a/libfrog/linux.c
+++ b/libfrog/linux.c
@@ -11,6 +11,7 @@
 
 #include "libxfs_priv.h"
 #include "xfs_fs.h"
+#include "init.h"
 
 extern char *progname;
 static int max_block_alignment;
diff --git a/libxfs/init.h b/libxfs/init.h
index e0b5091..2cda895 100644
--- a/libxfs/init.h
+++ b/libxfs/init.h
@@ -19,5 +19,7 @@ extern char *platform_findblockpath (char *path);
 extern int platform_direct_blockdev (void);
 extern int platform_align_blockdev (void);
 extern unsigned long platform_physmem(void);	/* in kilobytes */
+extern void platform_findsizes(char *path, int fd, long long *sz, int *bsz);
+extern int platform_nproc(void);
 
 #endif	/* LIBXFS_INIT_H */
-- 
1.8.3.1

[PATCH 08/18] xfs_io: include io.h to silence static function warnings

[Eric Sandeen]

io.h exports functions from io/getrusage.c and io/log_writes.c, so include
it from these files to silence warnings about potentially static functions
(getrusage_init, log_writes_init) in sparse.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 io/getrusage.c  | 1 +
 io/log_writes.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/io/getrusage.c b/io/getrusage.c
index ba704b7..6962913 100644
--- a/io/getrusage.c
+++ b/io/getrusage.c
@@ -9,6 +9,7 @@
 #include <sys/time.h>
 #include <sys/resource.h>
 #include "init.h"
+#include "io.h"
 
 static cmdinfo_t getrusage_cmd;
 
diff --git a/io/log_writes.c b/io/log_writes.c
index 114f818..9c2285f 100644
--- a/io/log_writes.c
+++ b/io/log_writes.c
@@ -8,6 +8,7 @@
 #include <libdevmapper.h>
 #include "command.h"
 #include "init.h"
+#include "io.h"
 
 static cmdinfo_t log_writes_cmd;
 
-- 
1.8.3.1

[PATCH 09/18] libxfs: silence sparse static function warnings in util.c

[Eric Sandeen]

libxfs.h exports functions defined in util.c, so include that file to
silence sparse warnings about potential static functions.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/util.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libxfs/util.c b/libxfs/util.c
index ffd2650..bd41404 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -5,6 +5,7 @@
  */
 
 #include "libxfs_priv.h"
+#include "libxfs.h"
 #include "libxfs_io.h"
 #include "init.h"
 #include "xfs_fs.h"
-- 
1.8.3.1

[PATCH 10/18] avl64: export avl64_firstino / avl64_firstino from avl64.h

[Eric Sandeen]

These are flagged by the sparse checker as possibly static.
They are actually not used at this point; avl64.c has a few unused
functions and/or ones that could be made static, but for now just silence
the warnings, and save deeper surgery for later.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 include/avl64.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/avl64.h b/include/avl64.h
index 7a66883..4042f6c 100644
--- a/include/avl64.h
+++ b/include/avl64.h
@@ -69,6 +69,12 @@ avl64_insert_immediate(
 	avl64node_t *afterp,
 	avl64node_t *newnode);
 
+avl64node_t *
+avl64_firstino(avl64node_t *root);
+
+avl64node_t *
+avl64_lastino(avl64node_t *root);
+
 void
 avl64_init_tree(
 	avl64tree_desc_t  *tree,
-- 
1.8.3.1

[PATCH 11/18] xfsprogs: misc static function warning fixes

[Eric Sandeen]

The last handful of static symbol warning cleanups for misc remaining
issues.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libxfs/init.h     | 1 +
 libxfs/rdwr.c     | 2 --
 repair/phase2.c   | 2 --
 repair/scan.h     | 2 ++
 scrub/xfs_scrub.h | 2 ++
 5 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/libxfs/init.h b/libxfs/init.h
index 2cda895..b23e493 100644
--- a/libxfs/init.h
+++ b/libxfs/init.h
@@ -7,6 +7,7 @@
 #define LIBXFS_INIT_H
 
 struct stat;
+extern int     use_xfs_buf_lock;
 
 extern int platform_check_ismounted (char *path, char *block,
 					struct stat *sptr, int verbose);
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index e580aba..a00360e 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -712,8 +712,6 @@ struct list_head	lock_buf_list = {&lock_buf_list, &lock_buf_list};
 int			lock_buf_count = 0;
 #endif
 
-extern int     use_xfs_buf_lock;
-
 static struct xfs_buf *
 __cache_lookup(struct xfs_bufkey *key, unsigned int flags)
 {
diff --git a/repair/phase2.c b/repair/phase2.c
index b92ceb8..4bd6c63 100644
--- a/repair/phase2.c
+++ b/repair/phase2.c
@@ -15,8 +15,6 @@
 #include "progress.h"
 #include "scan.h"
 
-void	set_mp(xfs_mount_t *mpp);
-
 /* workaround craziness in the xlog routines */
 int xlog_recover_do_trans(struct xlog *log, xlog_recover_t *t, int p)
 {
diff --git a/repair/scan.h b/repair/scan.h
index aa7c38d..bf1ee83 100644
--- a/repair/scan.h
+++ b/repair/scan.h
@@ -8,6 +8,8 @@
 
 struct blkmap;
 
+void set_mp(xfs_mount_t *mpp);
+
 int scan_lbtree(
 	xfs_fsblock_t	root,
 	int		nlevels,
diff --git a/scrub/xfs_scrub.h b/scrub/xfs_scrub.h
index 8b80c79..a961d8f 100644
--- a/scrub/xfs_scrub.h
+++ b/scrub/xfs_scrub.h
@@ -6,6 +6,8 @@
 #ifndef XFS_SCRUB_XFS_SCRUB_H_
 #define XFS_SCRUB_XFS_SCRUB_H_
 
+extern char *progname;
+
 #define _PATH_PROC_MOUNTS	"/proc/mounts"
 
 extern unsigned int		nr_threads;
-- 
1.8.3.1

[PATCH 12/18] xfsprogs: don't shadow global libxfs_init x variable

[Eric Sandeen]

We use the variable 'x' for the global libxfs_init structure, but several
other functions think 'x' is a nice convenient local var too, and sparse
complains.  Rename these local variables (or in some cases re-use existing
loop counter vars when x was used for this purpose).

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 db/check.c          | 15 +++++++--------
 logprint/log_misc.c | 31 ++++++++++++++++---------------
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/db/check.c b/db/check.c
index 683e176..352aab3 100644
--- a/db/check.c
+++ b/db/check.c
@@ -2050,7 +2050,7 @@ process_block_dir_v2(
 	int		nex;
 	xfs_ino_t	parent;
 	int		v;
-	int		x;
+	int		i;
 
 	nex = blkmap_getn(blkmap, 0, mp->m_dir_geo->fsbcount, &bmp);
 	v = id->ilist || verbose;
@@ -2067,9 +2067,9 @@ process_block_dir_v2(
 		make_bbmap(&bbmap, nex, bmp);
 	set_cur(&typtab[TYP_DIR2], XFS_FSB_TO_DADDR(mp, bmp->startblock),
 		mp->m_dir_geo->fsbcount * blkbb, DB_RING_IGN, nex > 1 ? &bbmap : NULL);
-	for (x = 0; !v && x < nex; x++) {
-		for (b = bmp[x].startblock;
-		     !v && b < bmp[x].startblock + bmp[x].blockcount;
+	for (i = 0; !v && i < nex; i++) {
+		for (b = bmp[i].startblock;
+		     !v && b < bmp[i].startblock + bmp[i].blockcount;
 		     b++)
 			v = CHECK_BLIST(b);
 	}
@@ -2998,7 +2998,6 @@ process_leaf_node_dir_v2(
 	int			t = 0;
 	int			v;
 	int			v2;
-	int			x;
 
 	v2 = verbose || id->ilist;
 	v = parent = 0;
@@ -3012,9 +3011,9 @@ process_leaf_node_dir_v2(
 	while ((dbno = blkmap_next_off(blkmap, dbno, &t)) != NULLFILEOFF) {
 		nex = blkmap_getn(blkmap, dbno, mp->m_dir_geo->fsbcount, &bmp);
 		ASSERT(nex > 0);
-		for (v = v2, x = 0; !v && x < nex; x++) {
-			for (b = bmp[x].startblock;
-			     !v && b < bmp[x].startblock + bmp[x].blockcount;
+		for (v = v2, i = 0; !v && i < nex; i++) {
+			for (b = bmp[i].startblock;
+			     !v && b < bmp[i].startblock + bmp[i].blockcount;
 			     b++)
 				v = CHECK_BLIST(b);
 		}
diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index e29366a..99d9920 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -192,7 +192,6 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
     int64_t			 blkno;
     xfs_buf_log_format_t lbuf;
     int			 size, blen, map_size, struct_size;
-    __be64		 x, y;
     unsigned short	 flags;
 
     /*
@@ -247,20 +246,22 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
 		if (be32_to_cpu(head->oh_len) < 4*8) {
 			printf(_("Out of space\n"));
 		} else {
+			__be64		 a, b;
+
 			printf("\n");
 			/*
 			 * memmove because *ptr may not be 8-byte aligned
 			 */
-			memmove(&x, *ptr, sizeof(__be64));
-			memmove(&y, *ptr+8, sizeof(__be64));
-		       printf(_("icount: %llu  ifree: %llu  "),
-			       (unsigned long long) be64_to_cpu(x),
-			       (unsigned long long) be64_to_cpu(y));
-			memmove(&x, *ptr+16, sizeof(__be64));
-			memmove(&y, *ptr+24, sizeof(__be64));
-		       printf(_("fdblks: %llu  frext: %llu\n"),
-			       (unsigned long long) be64_to_cpu(x),
-			       (unsigned long long) be64_to_cpu(y));
+			memmove(&a, *ptr, sizeof(__be64));
+			memmove(&b, *ptr+8, sizeof(__be64));
+			printf(_("icount: %llu  ifree: %llu  "),
+			       (unsigned long long) be64_to_cpu(a),
+			       (unsigned long long) be64_to_cpu(b));
+			memmove(&a, *ptr+16, sizeof(__be64));
+			memmove(&b, *ptr+24, sizeof(__be64));
+			printf(_("fdblks: %llu  frext: %llu\n"),
+			       (unsigned long long) be64_to_cpu(a),
+			       (unsigned long long) be64_to_cpu(b));
 		}
 		super_block = 0;
 	} else if (be32_to_cpu(*(__be32 *)(*ptr)) == XFS_AGI_MAGIC) {
@@ -1185,7 +1186,7 @@ xlog_print_extended_headers(
 	int 			num_hdrs;
 	int 			num_required;
 	char			xhbuf[XLOG_HEADER_SIZE];
-	xlog_rec_ext_header_t	*x;
+	xlog_rec_ext_header_t	*xhdr;
 
 	num_required = howmany(len, XLOG_HEADER_CYCLE_SIZE);
 	num_hdrs = be32_to_cpu(hdr->h_size) / XLOG_HEADER_CYCLE_SIZE;
@@ -1210,7 +1211,7 @@ xlog_print_extended_headers(
 	*ret_num_hdrs = num_hdrs;
 
 	/* don't include 1st header */
-	for (i = 1, x = *ret_xhdrs; i < num_hdrs; i++, (*blkno)++, x++) {
+	for (i = 1, xhdr = *ret_xhdrs; i < num_hdrs; i++, (*blkno)++, xhdr++) {
 	    /* read one extra header blk */
 	    if (read(fd, xhbuf, 512) == 0) {
 		printf(_("%s: physical end of log\n"), progname);
@@ -1240,9 +1241,9 @@ xlog_print_extended_headers(
 	     * will look asymmetric with the 1 hdr normal case
 	     * which does endian coversion on access.
 	     */
-	    x->xh_cycle = ((xlog_rec_ext_header_t*)xhbuf)->xh_cycle;
+	    xhdr->xh_cycle = ((xlog_rec_ext_header_t*)xhbuf)->xh_cycle;
 	    for (j = 0; j < XLOG_HEADER_CYCLE_SIZE / BBSIZE; j++) {
-		x->xh_cycle_data[j] =
+		xhdr->xh_cycle_data[j] =
 		    ((xlog_rec_ext_header_t*)xhbuf)->xh_cycle_data[j];
 	    }
 	}
-- 
1.8.3.1

[PATCH 13/18] xfs_io: rename global buffer variable

[Eric Sandeen]

"buffer" is a pretty poor name for a global variable, and leads to
shadow variable warnings from sparse when other functions (reasonably)
think it's a nice local variable name.

Rename it to io_buffer for less namespace pollution, and rename
buffersize to io_buffersize to go with it.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 io/io.h     |  4 ++--
 io/mmap.c   |  6 +++---
 io/pread.c  | 56 ++++++++++++++++++++++++++++----------------------------
 io/pwrite.c | 28 ++++++++++++++--------------
 4 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/io/io.h b/io/io.h
index 9278ad0..bc1e806 100644
--- a/io/io.h
+++ b/io/io.h
@@ -79,8 +79,8 @@ extern void		printxattr(uint, int, int, const char *, int, int);
 extern unsigned int	recurse_all;
 extern unsigned int	recurse_dir;
 
-extern void		*buffer;
-extern size_t		buffersize;
+extern void		*io_buffer;
+extern size_t		io_buffersize;
 extern int		vectors;
 extern struct iovec	*iov;
 extern int		alloc_buffer(size_t, int, unsigned int);
diff --git a/io/mmap.c b/io/mmap.c
index 44749bb..f9383e5 100644
--- a/io/mmap.c
+++ b/io/mmap.c
@@ -436,7 +436,7 @@ mread_f(
 
 	if (alloc_buffer(pagesize, 0, 0) < 0)
 		return 0;
-	bp = (char *)buffer;
+	bp = (char *)io_buffer;
 
 	dumplen = length % pagesize;
 	if (!dumplen)
@@ -451,7 +451,7 @@ mread_f(
 					dump_buffer(printoffset, dumplen);
 					printoffset += dumplen;
 				}
-				bp = (char *)buffer;
+				bp = (char *)io_buffer;
 				dumplen = pagesize;
 				cnt = 0;
 			} else {
@@ -466,7 +466,7 @@ mread_f(
 				if (dump)
 					dump_buffer(printoffset + tmp -
 						(dumplen - 1), dumplen);
-				bp = (char *)buffer;
+				bp = (char *)io_buffer;
 				dumplen = pagesize;
 				cnt = 0;
 			} else {
diff --git a/io/pread.c b/io/pread.c
index e573377..1b4352b 100644
--- a/io/pread.c
+++ b/io/pread.c
@@ -47,9 +47,9 @@ pread_help(void)
 "\n"));
 }
 
-void	*buffer;
+void	*io_buffer;
 size_t	highwater;
-size_t	buffersize;
+size_t	io_buffersize;
 int	vectors;
 struct iovec *iov;
 
@@ -65,7 +65,7 @@ alloc_iovec(
 	if (!iov)
 		return -1;
 
-	buffersize = 0;
+	io_buffersize = 0;
 	for (i = 0; i < vectors; i++) {
 		iov[i].iov_base = memalign(pagesize, bsize);
 		if (!iov[i].iov_base) {
@@ -76,7 +76,7 @@ alloc_iovec(
 		if (!uflag)
 			memset(iov[i].iov_base, seed, bsize);
 	}
-	buffersize = bsize * vectors;
+	io_buffersize = bsize * vectors;
 	return 0;
 unwind:
 	for( ; i >= 0; i--)
@@ -96,19 +96,19 @@ alloc_buffer(
 		return alloc_iovec(bsize, uflag, seed);
 
 	if (bsize > highwater) {
-		if (buffer)
-			free(buffer);
-		buffer = memalign(pagesize, bsize);
-		if (!buffer) {
+		if (io_buffer)
+			free(io_buffer);
+		io_buffer = memalign(pagesize, bsize);
+		if (!io_buffer) {
 			perror("memalign");
-			highwater = buffersize = 0;
+			highwater = io_buffersize = 0;
 			return -1;
 		}
 		highwater = bsize;
 	}
-	buffersize = bsize;
+	io_buffersize = bsize;
 	if (!uflag)
-		memset(buffer, seed, buffersize);
+		memset(io_buffer, seed, io_buffersize);
 	return 0;
 }
 
@@ -146,7 +146,7 @@ dump_buffer(
 	int		i, l;
 
 	if (!vectors) {
-		__dump_buffer(buffer, offset, len);
+		__dump_buffer(io_buffer, offset, len);
 		return;
 	}
 
@@ -171,7 +171,7 @@ do_preadv(
 	ssize_t		bytes = 0;
 
 	/* trim the iovec if necessary */
-	if (count < buffersize) {
+	if (count < io_buffersize) {
 		size_t	len = 0;
 		while (len + iov[vecs].iov_len < count) {
 			len += iov[vecs].iov_len;
@@ -203,7 +203,7 @@ do_pread(
 	size_t		buffer_size)
 {
 	if (!vectors)
-		return pread(fd, buffer, min(count, buffer_size), offset);
+		return pread(fd, io_buffer, min(count, buffer_size), offset);
 
 	return do_preadv(fd, offset, count);
 }
@@ -224,22 +224,22 @@ read_random(
 	srandom(seed);
 	end = lseek(fd, 0, SEEK_END);
 	offset = (eof || offset > end) ? end : offset;
-	if ((bytes = (offset % buffersize)))
+	if ((bytes = (offset % io_buffersize)))
 		offset -= bytes;
 	offset = max(0, offset);
-	if ((bytes = (count % buffersize)))
+	if ((bytes = (count % io_buffersize)))
 		count += bytes;
-	count = max(buffersize, count);
-	range = count - buffersize;
+	count = max(io_buffersize, count);
+	range = count - io_buffersize;
 
 	*total = 0;
 	while (count > 0) {
 		if (range)
-			off = ((offset + (random() % range)) / buffersize) *
-				buffersize;
+			off = ((offset + (random() % range)) / io_buffersize) *
+				io_buffersize;
 		else
 			off = offset;
-		bytes = do_pread(fd, off, buffersize, buffersize);
+		bytes = do_pread(fd, off, io_buffersize, io_buffersize);
 		if (bytes == 0)
 			break;
 		if (bytes < 0) {
@@ -248,7 +248,7 @@ read_random(
 		}
 		ops++;
 		*total += bytes;
-		if (bytes < buffersize)
+		if (bytes < io_buffersize)
 			break;
 		count -= bytes;
 	}
@@ -279,9 +279,9 @@ read_backward(
 	*offset = off;
 
 	/* Do initial unaligned read if needed */
-	if ((bytes_requested = (off % buffersize))) {
+	if ((bytes_requested = (off % io_buffersize))) {
 		off -= bytes_requested;
-		bytes = do_pread(fd, off, bytes_requested, buffersize);
+		bytes = do_pread(fd, off, bytes_requested, io_buffersize);
 		if (bytes == 0)
 			return ops;
 		if (bytes < 0) {
@@ -297,9 +297,9 @@ read_backward(
 
 	/* Iterate backward through the rest of the range */
 	while (cnt > end) {
-		bytes_requested = min(cnt, buffersize);
+		bytes_requested = min(cnt, io_buffersize);
 		off -= bytes_requested;
-		bytes = do_pread(fd, off, cnt, buffersize);
+		bytes = do_pread(fd, off, cnt, io_buffersize);
 		if (bytes == 0)
 			break;
 		if (bytes < 0) {
@@ -330,7 +330,7 @@ read_forward(
 
 	*total = 0;
 	while (count > 0 || eof) {
-		bytes = do_pread(fd, offset, count, buffersize);
+		bytes = do_pread(fd, offset, count, io_buffersize);
 		if (bytes == 0)
 			break;
 		if (bytes < 0) {
@@ -341,7 +341,7 @@ read_forward(
 		if (verbose)
 			dump_buffer(offset, bytes);
 		*total += bytes;
-		if (onlyone || bytes < min(count, buffersize))
+		if (onlyone || bytes < min(count, io_buffersize))
 			break;
 		offset += bytes;
 		count -= bytes;
diff --git a/io/pwrite.c b/io/pwrite.c
index 34235ca..ccf14be 100644
--- a/io/pwrite.c
+++ b/io/pwrite.c
@@ -62,7 +62,7 @@ do_pwritev(
 	ssize_t bytes = 0;
 
 	/* trim the iovec if necessary */
-	if (count < buffersize) {
+	if (count < io_buffersize) {
 		size_t	len = 0;
 		while (len + iov[vecs].iov_len < count) {
 			len += iov[vecs].iov_len;
@@ -102,7 +102,7 @@ do_pwrite(
 	int		pwritev2_flags)
 {
 	if (!vectors)
-		return pwrite(fd, buffer, min(count, buffer_size), offset);
+		return pwrite(fd, io_buffer, min(count, buffer_size), offset);
 
 	return do_pwritev(fd, offset, count, pwritev2_flags);
 }
@@ -120,22 +120,22 @@ write_random(
 	int		ops = 0;
 
 	srandom(seed);
-	if ((bytes = (offset % buffersize)))
+	if ((bytes = (offset % io_buffersize)))
 		offset -= bytes;
 	offset = max(0, offset);
-	if ((bytes = (count % buffersize)))
+	if ((bytes = (count % io_buffersize)))
 		count += bytes;
-	count = max(buffersize, count);
-	range = count - buffersize;
+	count = max(io_buffersize, count);
+	range = count - io_buffersize;
 
 	*total = 0;
 	while (count > 0) {
 		if (range)
-			off = ((offset + (random() % range)) / buffersize) *
-				buffersize;
+			off = ((offset + (random() % range)) / io_buffersize) *
+				io_buffersize;
 		else
 			off = offset;
-		bytes = do_pwrite(file->fd, off, buffersize, buffersize,
+		bytes = do_pwrite(file->fd, off, io_buffersize, io_buffersize,
 				pwritev2_flags);
 		if (bytes == 0)
 			break;
@@ -145,7 +145,7 @@ write_random(
 		}
 		ops++;
 		*total += bytes;
-		if (bytes < buffersize)
+		if (bytes < io_buffersize)
 			break;
 		count -= bytes;
 	}
@@ -172,10 +172,10 @@ write_backward(
 	*count = cnt;
 
 	/* Do initial unaligned write if needed */
-	if ((bytes_requested = (off % buffersize))) {
+	if ((bytes_requested = (off % io_buffersize))) {
 		bytes_requested = min(cnt, bytes_requested);
 		off -= bytes_requested;
-		bytes = do_pwrite(file->fd, off, bytes_requested, buffersize,
+		bytes = do_pwrite(file->fd, off, bytes_requested, io_buffersize,
 				pwritev2_flags);
 		if (bytes == 0)
 			return ops;
@@ -192,9 +192,9 @@ write_backward(
 
 	/* Iterate backward through the rest of the range */
 	while (cnt > end) {
-		bytes_requested = min(cnt, buffersize);
+		bytes_requested = min(cnt, io_buffersize);
 		off -= bytes_requested;
-		bytes = do_pwrite(file->fd, off, cnt, buffersize,
+		bytes = do_pwrite(file->fd, off, cnt, io_buffersize,
 				pwritev2_flags);
 		if (bytes == 0)
 			break;
-- 
1.8.3.1

[PATCH 14/18] xfs_logprint: fix shadow var in xlog_print_trans_buffer

[Eric Sandeen]

xlog_print_trans_buffer takes 'i' as an argument, but then uses it later
as a local byte counter.  Give the local var more useful, non-shadow
name of "byte"

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 logprint/log_misc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/logprint/log_misc.c b/logprint/log_misc.c
index 99d9920..c325f04 100644
--- a/logprint/log_misc.c
+++ b/logprint/log_misc.c
@@ -395,15 +395,15 @@ xlog_print_trans_buffer(char **ptr, int len, int *i, int num_ops)
 		if (print_data) {
 			uint *dp  = (uint *)*ptr;
 			int  nums = be32_to_cpu(head->oh_len) >> 2;
-			int  i = 0;
+			int  byte = 0;
 
-			while (i < nums) {
-				if ((i % 8) == 0)
-					printf("%2x ", i);
+			while (byte < nums) {
+				if ((byte % 8) == 0)
+					printf("%2x ", byte);
 				printf("%8x ", *dp);
 				dp++;
-				i++;
-				if ((i % 8) == 0)
+				byte++;
+				if ((byte % 8) == 0)
 					printf("\n");
 			}
 			printf("\n");
-- 
1.8.3.1

[PATCH 15/18] xfs_repair: fix 'bno' shadow vars in scan.c

[Eric Sandeen]

scan.c has 3 functions which accept 'bno' as an argument, but then use a
local variable of the same name for a different purpose in an inner scope,
which causes sparse warnings.
Rename these inner-scope loop variables 'agbno' to fix this.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 repair/scan.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/repair/scan.c b/repair/scan.c
index 65a76e2..12ca314 100644
--- a/repair/scan.c
+++ b/repair/scan.c
@@ -738,7 +738,7 @@ _("%s freespace btree block claimed (state %d), agno %d, bno %d, suspect %d\n"),
 	}
 
 	for (i = 0; i < numrecs; i++)  {
-		xfs_agblock_t		bno = be32_to_cpu(pp[i]);
+		xfs_agblock_t		agbno = be32_to_cpu(pp[i]);
 
 		/*
 		 * XXX - put sibling detection right here.
@@ -749,17 +749,17 @@ _("%s freespace btree block claimed (state %d), agno %d, bno %d, suspect %d\n"),
 		 * pointer mismatch, try and extract as much data
 		 * as possible.
 		 */
-		if (bno != 0 && verify_agbno(mp, agno, bno)) {
+		if (agbno != 0 && verify_agbno(mp, agno, agbno)) {
 			switch (magic) {
 			case XFS_ABTB_CRC_MAGIC:
 			case XFS_ABTB_MAGIC:
-				scan_sbtree(bno, level, agno, suspect,
+				scan_sbtree(agbno, level, agno, suspect,
 					    scan_allocbt, 0, magic, priv,
 					    &xfs_allocbt_buf_ops);
 				break;
 			case XFS_ABTC_CRC_MAGIC:
 			case XFS_ABTC_MAGIC:
-				scan_sbtree(bno, level, agno, suspect,
+				scan_sbtree(agbno, level, agno, suspect,
 					    scan_allocbt, 0, magic, priv,
 					    &xfs_allocbt_buf_ops);
 				break;
@@ -1177,7 +1177,7 @@ advance:
 	}
 
 	for (i = 0; i < numrecs; i++)  {
-		xfs_agblock_t		bno = be32_to_cpu(pp[i]);
+		xfs_agblock_t		agbno = be32_to_cpu(pp[i]);
 
 		/*
 		 * XXX - put sibling detection right here.
@@ -1199,12 +1199,12 @@ advance:
 			/* Look for impossible flags. */
 			do_warn(
 	_("invalid flags in high key %u of %s btree block %u/%u\n"),
-				i, name, agno, bno);
+				i, name, agno, agbno);
 			continue;
 		}
 
-		if (bno != 0 && verify_agbno(mp, agno, bno)) {
-			scan_sbtree(bno, level, agno, suspect, scan_rmapbt, 0,
+		if (agbno != 0 && verify_agbno(mp, agno, agbno)) {
+			scan_sbtree(agbno, level, agno, suspect, scan_rmapbt, 0,
 				    magic, priv, &xfs_rmapbt_buf_ops);
 		}
 	}
@@ -1419,10 +1419,10 @@ _("extent (%u/%u) len %u claimed, state is %d\n"),
 	}
 
 	for (i = 0; i < numrecs; i++)  {
-		xfs_agblock_t		bno = be32_to_cpu(pp[i]);
+		xfs_agblock_t		agbno = be32_to_cpu(pp[i]);
 
-		if (bno != 0 && verify_agbno(mp, agno, bno)) {
-			scan_sbtree(bno, level, agno, suspect, scan_refcbt, 0,
+		if (agbno != 0 && verify_agbno(mp, agno, agbno)) {
+			scan_sbtree(agbno, level, agno, suspect, scan_refcbt, 0,
 				    magic, priv, &xfs_refcountbt_buf_ops);
 		}
 	}
-- 
1.8.3.1

[PATCH 16/18] xfs_scrub: remove shadow var from run_scrub_phases()

[Eric Sandeen]

The local nr_threads shadows a global var of the same name.
Use a local variable to avoid confusion (even though estimate_work()
may simply fill in the value of the global...)

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 scrub/xfs_scrub.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c
index 33b83a5..a789b6f 100644
--- a/scrub/xfs_scrub.c
+++ b/scrub/xfs_scrub.c
@@ -422,7 +422,6 @@ run_scrub_phases(
 	bool			moveon = true;
 	unsigned int		debug_phase = 0;
 	unsigned int		phase;
-	unsigned int		nr_threads;
 	int			rshift;
 
 	if (debug_tweak_on("XFS_SCRUB_PHASE"))
@@ -454,12 +453,14 @@ run_scrub_phases(
 		if (!moveon)
 			break;
 		if (sp->estimate_work) {
-			moveon = sp->estimate_work(ctx, &max_work, &nr_threads,
-					&rshift);
+			unsigned int		work_threads;
+
+			moveon = sp->estimate_work(ctx, &max_work,
+					&work_threads, &rshift);
 			if (!moveon)
 				break;
 			moveon = progress_init_phase(ctx, progress_fp, phase,
-					max_work, rshift, nr_threads);
+					max_work, rshift, work_threads);
 		} else {
 			moveon = progress_init_phase(ctx, NULL, phase, 0, 0, 0);
 		}
-- 
1.8.3.1

[PATCH 17/18] xfs_metadump: remove shadow variable

[Eric Sandeen]

"length" is used in 2 nested inner scopes, which is fairly unclear and
generates a sparse warning.  Rename the inner scope variable for clarity.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 db/metadump.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/db/metadump.c b/db/metadump.c
index cc2ae9a..8b8e472 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -1515,9 +1515,10 @@ process_dir_data_block(
 		dup = (xfs_dir2_data_unused_t *)ptr;
 
 		if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) {
-			int	length = be16_to_cpu(dup->length);
-			if (dir_offset + length > end_of_data ||
-			    !length || (length & (XFS_DIR2_DATA_ALIGN - 1))) {
+			int	free_length = be16_to_cpu(dup->length);
+			if (dir_offset + free_length > end_of_data ||
+			    !free_length ||
+			    (free_length & (XFS_DIR2_DATA_ALIGN - 1))) {
 				if (show_warnings)
 					print_warning(
 			"invalid length for dir free space in inode %llu",
@@ -1527,15 +1528,15 @@ process_dir_data_block(
 			if (be16_to_cpu(*xfs_dir2_data_unused_tag_p(dup)) !=
 					dir_offset)
 				return;
-			dir_offset += length;
-			ptr += length;
+			dir_offset += free_length;
+			ptr += free_length;
 			/*
 			 * Zero the unused space up to the tag - the tag is
 			 * actually at a variable offset, so zeroing &dup->tag
 			 * is zeroing the free space in between
 			 */
 			if (zero_stale_data) {
-				int zlen = length -
+				int zlen = free_length -
 						sizeof(xfs_dir2_data_unused_t);
 
 				if (zlen > 0) {
-- 
1.8.3.1

[PATCH 18/18] libfrog: change project entity variable scope to local/static

[Eric Sandeen]

The project quota code used a global variable "p" for getprent() and
getprpathent(), presumably to keep the interface analogous to getpwent()
etc.  However, other functions had their own local "p" which led to shadow
variable warnings from sparse.

Rather than a global, make it a static variable within the project
functions.  Same behavior, same interface, less confusion, and retains an
interface similar that of getpwent etc.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 libfrog/projects.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libfrog/projects.c b/libfrog/projects.c
index d4dda3f..91bc78f 100644
--- a/libfrog/projects.c
+++ b/libfrog/projects.c
@@ -15,12 +15,8 @@ char *projid_file;
 char *projects_file;
 
 static FILE *projects;
-static fs_project_t p;
-static char projects_buffer[512];
 
 static FILE *project_paths;
-static fs_project_path_t pp;
-static char project_paths_buffer[1024];
 
 void
 setprfiles(void)
@@ -64,8 +60,10 @@ endprpathent(void)
 fs_project_t *
 getprent(void)
 {
-	char	*idstart, *idend;
-	size_t	size = sizeof(projects_buffer) - 1;
+	static		fs_project_t p;
+	static char	projects_buffer[512];
+	char		*idstart, *idend;
+	size_t		size = sizeof(projects_buffer) - 1;
 
 	if (!projects)
 		return NULL;
@@ -125,6 +123,8 @@ getprprid(
 fs_project_path_t *
 getprpathent(void)
 {
+	static 		fs_project_path_t pp;
+	static char	project_paths_buffer[1024];
 	char		*nmstart, *nmend;
 	size_t		size = sizeof(project_paths_buffer) - 1;
 
-- 
1.8.3.1

Re: [PATCH 01/18] xfsprogs: enable sparse checking with "make C=[12]"

[Darrick J. Wong]

On Wed, Oct 10, 2018 at 03:01:05PM -0500, Eric Sandeen wrote:
> Enable "make C=1" or "make C=2" to do sparse checking.
> Blatantly ripped off from djwong's patch in e2fsprogs to do the same.
> 
> This is a bit simpler than redefining CC for the whole build, which
> requires extra commandline definitions and apparently is enough of a
> barrier that nobody's doing sparse checking.
> 
> Note, this requires unreleased sparse after v0.5, which enables the
> CHAR_BIT definition; otherwise it chokes.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  Makefile           | 14 ++++++++++++++
>  include/buildrules |  2 ++

Please update doc/sparse.txt...

--D

>  2 files changed, 16 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index d031a60..7f3e774 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -16,6 +16,20 @@ else
>    Q = @
>  endif
>  
> +CHECK=sparse
> +CHECK_OPTS=-Wsparse-all -Wno-transparent-union -Wno-return-void -Wno-undef \
> +	-Wno-non-pointer-null -D__linux__
> +ifeq ("$(C)", "2")
> +  CHECK_CMD=$(CHECK) $(CHECK_OPTS) -Wbitwise -D__CHECK_ENDIAN__
> +else
> +  ifeq ("$(C)", "1")
> +    CHECK_CMD=$(CHECK) $(CHECK_OPTS)
> +   else
> +    CHECK_CMD=@true
> +  endif
> +endif
> +export CHECK_CMD
> +
>  MAKEOPTS = --no-print-directory Q=$(Q)
>  
>  TOPDIR = .
> diff --git a/include/buildrules b/include/buildrules
> index c57fdfc..23fc866 100644
> --- a/include/buildrules
> +++ b/include/buildrules
> @@ -54,10 +54,12 @@ $(LTLIBRARY) : $(SUBDIRS) $(LTOBJECTS)
>  %.lo: %.c
>  	@echo "    [CC]     $@"
>  	$(Q)$(LTCOMPILE) -c $<
> +	$(Q)$(CHECK_CMD) $(CFLAGS) $<
>  else
>  %.o: %.c
>  	@echo "    [CC]     $@"
>  	$(Q)$(CC) $(CFLAGS) -c $<
> +	$(Q)$(CHECK_CMD) $(CFLAGS) $<
>  
>  endif
>  
> -- 
> 1.8.3.1
> 

Re: [PATCH 17/18] xfs_metadump: remove shadow variable

[Darrick J. Wong]

On Wed, Oct 10, 2018 at 03:01:21PM -0500, Eric Sandeen wrote:
> "length" is used in 2 nested inner scopes, which is fairly unclear and
> generates a sparse warning.  Rename the inner scope variable for clarity.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  db/metadump.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/db/metadump.c b/db/metadump.c
> index cc2ae9a..8b8e472 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -1515,9 +1515,10 @@ process_dir_data_block(
>  		dup = (xfs_dir2_data_unused_t *)ptr;
>  
>  		if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) {
> -			int	length = be16_to_cpu(dup->length);
> -			if (dir_offset + length > end_of_data ||
> -			    !length || (length & (XFS_DIR2_DATA_ALIGN - 1))) {
> +			int	free_length = be16_to_cpu(dup->length);

I know you're just changing names, but there ought to be a blank line
betweeen the variable declaration and the first line of code.

Looks fine otherwise.

--D

> +			if (dir_offset + free_length > end_of_data ||
> +			    !free_length ||
> +			    (free_length & (XFS_DIR2_DATA_ALIGN - 1))) {
>  				if (show_warnings)
>  					print_warning(
>  			"invalid length for dir free space in inode %llu",
> @@ -1527,15 +1528,15 @@ process_dir_data_block(
>  			if (be16_to_cpu(*xfs_dir2_data_unused_tag_p(dup)) !=
>  					dir_offset)
>  				return;
> -			dir_offset += length;
> -			ptr += length;
> +			dir_offset += free_length;
> +			ptr += free_length;
>  			/*
>  			 * Zero the unused space up to the tag - the tag is
>  			 * actually at a variable offset, so zeroing &dup->tag
>  			 * is zeroing the free space in between
>  			 */
>  			if (zero_stale_data) {
> -				int zlen = length -
> +				int zlen = free_length -
>  						sizeof(xfs_dir2_data_unused_t);
>  
>  				if (zlen > 0) {
> -- 
> 1.8.3.1
> 

Re: [PATCH 00/18] xfsprogs: finer-grained sparse fixes

[Darrick J. Wong]

On Wed, Oct 10, 2018 at 03:01:04PM -0500, Eric Sandeen wrote:
> hch asked for finer-grained patches for some of the sparse warning
> fixes, so here we go.
> 

For all the ones I didn't complain about,

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> Thanks,
> -Eric
> 

[PATCH 01/18 V2] xfsprogs: enable sparse checking with "make C=[12]"

[Eric Sandeen]

Enable "make C=1" or "make C=2" to do sparse checking.
Blatantly ripped off from djwong's patch in e2fsprogs to do the same.

This is a bit simpler than redefining CC for the whole build, which
requires extra commandline definitions and apparently is enough of a
barrier that nobody's doing sparse checking.

Note, this requires unreleased sparse after v0.5, which enables the
CHAR_BIT definition; otherwise it chokes.

Add information about these helpers to doc/sparse.txt

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/Makefile b/Makefile
index d031a60..7f3e774 100644
--- a/Makefile
+++ b/Makefile
@@ -16,6 +16,20 @@ else
   Q = @
 endif
 
+CHECK=sparse
+CHECK_OPTS=-Wsparse-all -Wno-transparent-union -Wno-return-void -Wno-undef \
+	-Wno-non-pointer-null -D__linux__
+ifeq ("$(C)", "2")
+  CHECK_CMD=$(CHECK) $(CHECK_OPTS) -Wbitwise -D__CHECK_ENDIAN__
+else
+  ifeq ("$(C)", "1")
+    CHECK_CMD=$(CHECK) $(CHECK_OPTS)
+   else
+    CHECK_CMD=@true
+  endif
+endif
+export CHECK_CMD
+
 MAKEOPTS = --no-print-directory Q=$(Q)
 
 TOPDIR = .
diff --git a/doc/sparse.txt b/doc/sparse.txt
index 36a34a0..d426739 100644
--- a/doc/sparse.txt
+++ b/doc/sparse.txt
@@ -5,9 +5,20 @@ to check the source code of the open source XFS commands and utilites
 First you need to install sparse, either from your distribution or from
 source as provided at http://www.kernel.org/pub/software/devel/sparse/.
 
-To simply build the xfsprogs source code while checking the source using
-sparse just set the compiler to cgcc, which is a wrapper that calls both
-sparse and gcc using:
+The xfsprogs Makefile has a convenient shortcut to running sparse, by setting
+the C ("check") variable on the make commandline.  To perform generic checks,
+
+	make C=1
+
+which checks with -Wsparse-all -Wno-transparent-union -Wno-return-void
+-Wno-undef -Wno-non-pointer-null, or to perform the bitwise checks, use
+
+	make C=2
+
+which checks with -Wbitwise -D__CHECK_ENDIAN__
+
+If you'd rather run sparse more manually, just set the compiler to cgcc,
+which is a wrapper that calls both sparse and gcc using:
 
 	CC=cgcc ./configure
 
diff --git a/include/buildrules b/include/buildrules
index c57fdfc..23fc866 100644
--- a/include/buildrules
+++ b/include/buildrules
@@ -54,10 +54,12 @@ $(LTLIBRARY) : $(SUBDIRS) $(LTOBJECTS)
 %.lo: %.c
 	@echo "    [CC]     $@"
 	$(Q)$(LTCOMPILE) -c $<
+	$(Q)$(CHECK_CMD) $(CFLAGS) $<
 else
 %.o: %.c
 	@echo "    [CC]     $@"
 	$(Q)$(CC) $(CFLAGS) -c $<
+	$(Q)$(CHECK_CMD) $(CFLAGS) $<
 
 endif
 

Re: [PATCH 01/18 V2] xfsprogs: enable sparse checking with "make C=[12]"

[Christoph Hellwig]

> -sparse and gcc using:
> +The xfsprogs Makefile has a convenient shortcut to running sparse, by setting
> +the C ("check") variable on the make commandline.  To perform generic checks,
> +
> +	make C=1
> +
> +which checks with -Wsparse-all -Wno-transparent-union -Wno-return-void
> +-Wno-undef -Wno-non-pointer-null, or to perform the bitwise checks, use
> +
> +	make C=2
> +
> +which checks with -Wbitwise -D__CHECK_ENDIAN__

Out of all the sparce checks applicable to xfsprogs endianess is by
far the most impotant one.  I don't think it makes any sense to ever
turn it off..

Re: [PATCH 02/18] xfs_db: convert single-bit bitfields to bools

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:06PM -0500, Eric Sandeen wrote:
> Sparse doesn't like signed single-bit bitfields, and they may as well
> just be booleans.
> 
> Fixes sparse warnings about this.

Looks good, although we could still do bool bitfields to save a tiny
bit of space if we areally cared:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 03/18] xfsprogs: minor endian annotation fixes

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:07PM -0500, Eric Sandeen wrote:
> No actual bugs, just quiet the sparse checker.
> 
> Fixes sparse warnings about this.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 04/18] xfsprogs: avoid redefinition of NBBY

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:08PM -0500, Eric Sandeen wrote:
> Include sys/param.h for NBBY definition.  Do this before our local
> guarded definition which is for platforms like android that don't have it,
> see commit:
> 
> c9a90185 xfsprogs: define NBBY if not defined by the system header files
> 
> Fixes sparse warnings about this redefinition.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 05/18] xfsprogs: include headers for extern variables

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:09PM -0500, Eric Sandeen wrote:
> Include headers which export functions for their matching C files so that
> they don't appear to be static to the sparse checker.
> 
> Fixes sparse warnings about this.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 06/18] libxfs: add several zone extern declarations to libxfs_priv.h

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:10PM -0500, Eric Sandeen wrote:
> Several zones have extern declarations in kernelspace headers we don't
> have in userspace.
> 
> Adding these to the libxfs_priv.h header silences sparse warnings about
> whether these should be static vars.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 07/18] libxfs: silence static warnings about platform_* functions

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:11PM -0500, Eric Sandeen wrote:
> Add all platform_* prototypes to init.h and include it in linux.c
> to silence sparse warnings about static functions.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 08/18] xfs_io: include io.h to silence static function warnings

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:12PM -0500, Eric Sandeen wrote:
> io.h exports functions from io/getrusage.c and io/log_writes.c, so include
> it from these files to silence warnings about potentially static functions
> (getrusage_init, log_writes_init) in sparse.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 09/18] libxfs: silence sparse static function warnings in util.c

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:13PM -0500, Eric Sandeen wrote:
> libxfs.h exports functions defined in util.c, so include that file to
> silence sparse warnings about potential static functions.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 10/18] avl64: export avl64_firstino / avl64_firstino from avl64.h

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:14PM -0500, Eric Sandeen wrote:
> These are flagged by the sparse checker as possibly static.
> They are actually not used at this point; avl64.c has a few unused
> functions and/or ones that could be made static, but for now just silence
> the warnings, and save deeper surgery for later.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

They've been unused for how long?  I'd favor just removing them, but
until that happens:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 11/18] xfsprogs: misc static function warning fixes

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:15PM -0500, Eric Sandeen wrote:
> The last handful of static symbol warning cleanups for misc remaining
> issues.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 12/18] xfsprogs: don't shadow global libxfs_init x variable

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:16PM -0500, Eric Sandeen wrote:
> We use the variable 'x' for the global libxfs_init structure, but several
> other functions think 'x' is a nice convenient local var too, and sparse
> complains.  Rename these local variables (or in some cases re-use existing
> loop counter vars when x was used for this purpose).
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 13/18] xfs_io: rename global buffer variable

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:17PM -0500, Eric Sandeen wrote:
> "buffer" is a pretty poor name for a global variable, and leads to
> shadow variable warnings from sparse when other functions (reasonably)
> think it's a nice local variable name.
> 
> Rename it to io_buffer for less namespace pollution, and rename
> buffersize to io_buffersize to go with it.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 14/18] xfs_logprint: fix shadow var in xlog_print_trans_buffer

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:18PM -0500, Eric Sandeen wrote:
> xlog_print_trans_buffer takes 'i' as an argument, but then uses it later
> as a local byte counter.  Give the local var more useful, non-shadow
> name of "byte"
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 15/18] xfs_repair: fix 'bno' shadow vars in scan.c

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:19PM -0500, Eric Sandeen wrote:
> scan.c has 3 functions which accept 'bno' as an argument, but then use a
> local variable of the same name for a different purpose in an inner scope,
> which causes sparse warnings.
> Rename these inner-scope loop variables 'agbno' to fix this.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 16/18] xfs_scrub: remove shadow var from run_scrub_phases()

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:20PM -0500, Eric Sandeen wrote:
> The local nr_threads shadows a global var of the same name.
> Use a local variable to avoid confusion (even though estimate_work()
> may simply fill in the value of the global...)
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 17/18] xfs_metadump: remove shadow variable

[Christoph Hellwig]

On Wed, Oct 10, 2018 at 03:01:21PM -0500, Eric Sandeen wrote:
> "length" is used in 2 nested inner scopes, which is fairly unclear and
> generates a sparse warning.  Rename the inner scope variable for clarity.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 01/18 V2] xfsprogs: enable sparse checking with "make C=[12]"

[Eric Sandeen]

On 10/11/18 12:57 AM, Christoph Hellwig wrote:
>> -sparse and gcc using:
>> +The xfsprogs Makefile has a convenient shortcut to running sparse, by setting
>> +the C ("check") variable on the make commandline.  To perform generic checks,
>> +
>> +	make C=1
>> +
>> +which checks with -Wsparse-all -Wno-transparent-union -Wno-return-void
>> +-Wno-undef -Wno-non-pointer-null, or to perform the bitwise checks, use
>> +
>> +	make C=2
>> +
>> +which checks with -Wbitwise -D__CHECK_ENDIAN__
> 
> Out of all the sparce checks applicable to xfsprogs endianess is by
> far the most impotant one.  I don't think it makes any sense to ever
> turn it off..

Oh... whoops, I thought I was emulating kernel behavior (by following e2fsprogs
behavior), but sadly no:

> Do a kernel make with "make C=1" to run sparse on all the C files that get
> recompiled, or use "make C=2" to run sparse on the files whether they need to
> be recompiled or not.  The latter is a fast way to check the whole tree if you
> have already built it.

Let me see if I can properly mimic that behavior, with endian checking baked
in for both.

Thanks for bringing it up,
-Eric