[RFC] btrfs-progs: fix sparse checking and warnings

[RFC] btrfs-progs: fix sparse checking and warnings

[Zach Brown]

Hi gang,

I was a little surprised to see that patch go by recently 
which fixed an endian bug.  I went to see how sparse
checking looked and it was.. broken.  I got it going
again in my Fedora environment.

Most of the patches are just cleanups, but there *were*
three real bugs lurking in all that sparse warning spam.
So I maintain that it's worth our time to keep it going
and fix warnings so that we have a chance of finding these
real bugs before users do.

I've only just built this and I haven't checked that it
doesn't break the build during bisection.  I wanted to
let people see it before getting too lost fiddling with it.
Dave, I'm happy to rework this as needed to get it integrated.

FWIW, it's worth running a locally built sparse from its
current git tree to avoid unknown attribute warnings.

-z

[PATCH 01/15] btrfs-progs: get C=1 sparse checking working again

[Zach Brown]

There were a few problems that were breaking sparse checking:

- We were defining CHECK_ENDIAN late in the environment, after
  linux/fs.h has been included which defines __force and __bitwise in
  confusing ways that conflict with ours.  Define it up with __CHECKER__
  so that linux/fs.h and our copy are acting on the same input.

- We had manually set a few of gcc's internal defines to give to sparse.
  It's easier to just ask gcc for all the defines it sets and hand those
  to sparse.

- We weren't passing the same *FLAGS to sparse as we were to CC.

- glibc has so many errors with FORTIFY turned on that sparse gives up
  and doesn't show us any errors from our code.  It's a questionable
  hack to always turn on FORTIFY ourselves, so we'll just not do that
  when building with sparse.

And add a nice '[SP]' quiet output line for sparse checks.

Signed-off-by: Zach Brown <zab@redhat.com>
---
 Makefile     | 28 +++++++++++++++++++++-------
 kerncompat.h |  1 -
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index fa8b34f..57cf3c8 100644
--- a/Makefile
+++ b/Makefile
@@ -1,7 +1,7 @@
 CC = gcc
 LN = ln
 AR = ar
-AM_CFLAGS = -Wall -D_FILE_OFFSET_BITS=64 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -DBTRFS_FLAT_INCLUDES -fPIC
+AM_CFLAGS = -Wall -D_FILE_OFFSET_BITS=64 -DBTRFS_FLAT_INCLUDES -fPIC
 CFLAGS = -g -O1
 objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
 	  root-tree.o dir-item.o file-item.o inode-item.o inode-map.o \
@@ -17,9 +17,6 @@ libbtrfs_headers = send-stream.h send-utils.h send.h rbtree.h btrfs-list.h \
 	       crc32c.h list.h kerncompat.h radix-tree.h extent-cache.h \
 	       extent_io.h ioctl.h ctree.h btrfsck.h
 
-CHECKFLAGS= -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ -Wbitwise \
-	    -Wuninitialized -Wshadow -Wundef
-
 INSTALL = install
 prefix ?= /usr/local
 bindir = $(prefix)/bin
@@ -70,17 +67,34 @@ lib_links = libbtrfs.so.0 libbtrfs.so
 headers = $(libbtrfs_headers)
 
 # make C=1 to enable sparse
+check_defs := .cc-defines.h 
 ifdef C
-	check = sparse $(CHECKFLAGS)
+	#
+	# We're trying to use sparse against glibc headers which go wild
+	# trying to use internal compiler macros to test features.  We
+	# copy gcc's and give them to sparse.  But not __SIZE_TYPE__
+	# 'cause sparse defines that one.
+	#
+	dummy := $(shell $(CC) -dM -E -x c - < /dev/null | \
+			grep -v __SIZE_TYPE__ > $(check_defs))
+	check = sparse -include $(check_defs) -D__CHECKER__ \
+		-D__CHECK_ENDIAN__ -Wbitwise -Wuninitialized -Wshadow -Wundef
+	check_echo = echo
+	# don't use FORTIFY with sparse because glibc with FORTIFY can
+	# generate so many sparse errors that sparse stops parsing,
+	# which masks real errors that we want to see.
 else
 	check = true
+	check_echo = true
+	AM_CFLAGS += -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
 endif
 
 %.o.d: %.c
 	$(Q)$(CC) -MM -MG -MF $@ -MT $(@:.o.d=.o) -MT $(@:.o.d=.static.o) -MT $@ $(AM_CFLAGS) $(CFLAGS) $<
 
 .c.o:
-	$(Q)$(check) $<
+	@$(check_echo) "    [SP]     $@"
+	$(Q)$(check) $(AM_CFLAGS) $(CFLAGS) $<
 	@echo "    [CC]     $@"
 	$(Q)$(CC) $(AM_CFLAGS) $(CFLAGS) -c $<
 
@@ -189,7 +203,7 @@ clean :
 	$(Q)rm -f $(progs) cscope.out *.o *.o.d btrfs-convert btrfs-image btrfs-select-super \
 	      btrfs-zero-log btrfstune dir-test ioctl-test quick-test send-test btrfsck \
 	      btrfs.static mkfs.btrfs.static btrfs-calc-size \
-	      version.h \
+	      version.h $(check_defs)\
 	      $(libs) $(lib_links)
 	$(Q)$(MAKE) $(MAKEOPTS) -C man $@
 
diff --git a/kerncompat.h b/kerncompat.h
index ebb54b1..4d78288 100644
--- a/kerncompat.h
+++ b/kerncompat.h
@@ -241,7 +241,6 @@ static inline long IS_ERR(const void *ptr)
         const typeof( ((type *)0)->member ) *__mptr = (ptr);    \
 	        (type *)( (char *)__mptr - offsetof(type,member) );})
 #ifdef __CHECKER__
-#define __CHECK_ENDIAN__
 #define __bitwise __bitwise__
 #else
 #define __bitwise
-- 
1.7.11.7

Re: [PATCH 01/15] btrfs-progs: get C=1 sparse checking working again

[David Sterba]

On Wed, Aug 14, 2013 at 04:16:31PM -0700, Zach Brown wrote:
> There were a few problems that were breaking sparse checking:
> 
> - We were defining CHECK_ENDIAN late in the environment, after
>   linux/fs.h has been included which defines __force and __bitwise in
>   confusing ways that conflict with ours.  Define it up with __CHECKER__
>   so that linux/fs.h and our copy are acting on the same input.
> 
> - We had manually set a few of gcc's internal defines to give to sparse.
>   It's easier to just ask gcc for all the defines it sets and hand those
>   to sparse.
> 
> - We weren't passing the same *FLAGS to sparse as we were to CC.
> 
> - glibc has so many errors with FORTIFY turned on that sparse gives up
>   and doesn't show us any errors from our code.  It's a questionable
>   hack to always turn on FORTIFY ourselves, so we'll just not do that
>   when building with sparse.
> 
> And add a nice '[SP]' quiet output line for sparse checks.

Very nice, thanks.

I'm getting this error, for each built object file:

$ make V=1 C=1
    [SP]     ctree.o
sparse -include .cc-defines.h  -D__CHECKER__ -D__CHECK_ENDIAN__ -Wbitwise -Wuninitialized -Wshadow -Wundef -Wall -D_FILE_OFFSET_BITS=64 -DBTRFS_FLAT_INCLUDES -fPIC -g -O1 ctree.c
/usr/include/stdio.h:33:12: error: unable to open 'stddef.h'
make: *** [ctree.o] Error 1

Re: [PATCH 01/15] btrfs-progs: get C=1 sparse checking working again

[Zach Brown]

> Very nice, thanks.
> 
> I'm getting this error, for each built object file:
> 
> $ make V=1 C=1
>     [SP]     ctree.o

Heh, I goofed when building the echo and actual rules, might as well
update that to $< to have it output .c?

        @$(check_echo) "    [SP]     $@"
        $(Q)$(check) $(AM_CFLAGS) $(CFLAGS) $<

:)

> sparse -include .cc-defines.h  -D__CHECKER__ -D__CHECK_ENDIAN__ -Wbitwise -Wuninitialized -Wshadow -Wundef -Wall -D_FILE_OFFSET_BITS=64 -DBTRFS_FLAT_INCLUDES -fPIC -g -O1 ctree.c
> /usr/include/stdio.h:33:12: error: unable to open 'stddef.h'
> make: *** [ctree.o] Error 1

*nod*.  I also ran in to this on another box.  Indeed, as you said in a
later email, this comes from older sparse not knowing to add the
internal gcc include installation paths to its search path.  You can
hack this yourself:

gcc_install := "$(shell gcc --print-search | \
                awk '($$1 == "install:"){print $$2}')/install"

	sparse -I$(gcc_install)

But honestly, I'm not sure that we should bother, given all the other
problems with older sparse.  It seems acceptable to ask people to run
the latest git snapshot.  It's easy.

- z

Re: [PATCH 01/15] btrfs-progs: get C=1 sparse checking working again

[David Sterba]

On Fri, Aug 30, 2013 at 02:03:28PM -0700, Zach Brown wrote:
> Heh, I goofed when building the echo and actual rules, might as well
> update that to $< to have it output .c?
> 
>         @$(check_echo) "    [SP]     $@"
>         $(Q)$(check) $(AM_CFLAGS) $(CFLAGS) $<

Yeah, looks better with .c

> > sparse -include .cc-defines.h  -D__CHECKER__ -D__CHECK_ENDIAN__ -Wbitwise -Wuninitialized -Wshadow -Wundef -Wall -D_FILE_OFFSET_BITS=64 -DBTRFS_FLAT_INCLUDES -fPIC -g -O1 ctree.c
> > /usr/include/stdio.h:33:12: error: unable to open 'stddef.h'
> > make: *** [ctree.o] Error 1
> 
> *nod*.  I also ran in to this on another box.  Indeed, as you said in a
> later email, this comes from older sparse not knowing to add the
> internal gcc include installation paths to its search path.  You can
> hack this yourself:
> 
> gcc_install := "$(shell gcc --print-search | \
>                 awk '($$1 == "install:"){print $$2}')/install"
> 
> 	sparse -I$(gcc_install)
> 
> But honestly, I'm not sure that we should bother, given all the other
> problems with older sparse.  It seems acceptable to ask people to run
> the latest git snapshot.  It's easy.

Ack.

I've reinstalled from the distro provided package with tag
sparse-20130425 and it worked.

david

[PATCH 02/15] btrfs-progs: remove __CHECKER__ from main code

[Zach Brown]

__CHECKER__ is only for the type juggling used to tell sparse which
types need conversion between address spaces.  It is not OK to use to
change the code that gets checked to avoid bugs elsewhere in the build
infrastructure.  We want to check the code that builds when the checker
isn't enabled.

Signed-off-by: Zach Brown <zab@redhat.com>
---
 btrfs-convert.c | 2 --
 btrfs-list.c    | 2 --
 cmds-device.c   | 9 ---------
 mkfs.c          | 3 ---
 utils.c         | 7 -------
 5 files changed, 23 deletions(-)

diff --git a/btrfs-convert.c b/btrfs-convert.c
index a4608ec..9a7da57 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -21,10 +21,8 @@
 
 #include "kerncompat.h"
 
-#ifndef __CHECKER__
 #include <sys/ioctl.h>
 #include <sys/mount.h>
-#endif
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/types.h>
diff --git a/btrfs-list.c b/btrfs-list.c
index 072a592..214cb45 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -17,11 +17,9 @@
  */
 
 #define _GNU_SOURCE
-#ifndef __CHECKER__
 #include <sys/ioctl.h>
 #include <sys/mount.h>
 #include "ioctl.h"
-#endif
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/types.h>
diff --git a/cmds-device.c b/cmds-device.c
index be2aaff..48ac526 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -30,15 +30,6 @@
 
 #include "commands.h"
 
-/* FIXME - imported cruft, fix sparse errors and warnings */
-#ifdef __CHECKER__
-#define BLKGETSIZE64 0
-#define BTRFS_IOC_SNAP_CREATE_V2 0
-#define BTRFS_VOL_NAME_MAX 255
-struct btrfs_ioctl_vol_args { char name[BTRFS_VOL_NAME_MAX]; };
-static inline int ioctl(int fd, int define, void *arg) { return 0; }
-#endif
-
 static const char * const device_cmd_group_usage[] = {
 	"btrfs device <command> [<args>]",
 	NULL
diff --git a/mkfs.c b/mkfs.c
index 8183879..5724dec 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -21,12 +21,9 @@
 
 #include "kerncompat.h"
 
-#ifndef __CHECKER__
 #include <sys/ioctl.h>
 #include <sys/mount.h>
 #include "ioctl.h"
-#endif
-
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/types.h>
diff --git a/utils.c b/utils.c
index 86ee948..e0417cb 100644
--- a/utils.c
+++ b/utils.c
@@ -24,10 +24,8 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#ifndef __CHECKER__
 #include <sys/ioctl.h>
 #include <sys/mount.h>
-#endif
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <uuid/uuid.h>
@@ -50,11 +48,6 @@
 #include "volumes.h"
 #include "ioctl.h"
 
-#ifdef __CHECKER__
-#define BLKGETSIZE64 0
-static inline int ioctl(int fd, int define, u64 *size) { return 0; }
-#endif
-
 #ifndef BLKDISCARD
 #define BLKDISCARD	_IO(0x12,119)
 #endif
-- 
1.7.11.7

[PATCH 03/15] btrfs-progs: add ULL to u64 constant

[Zach Brown]

This silences a sparse warning:

	warning: constant 0x4D5F53665248425F is so big it is long

from

  commit 52162700bb59663add809a6465ce2769d80b3664
  Author: Zach Brown <zab@redhat.com>
  Date:   Thu Jan 17 11:54:47 2013 -0800

      btrfs-progs: treat super.magic as an le64

High fives, past me!

Signed-off-by: Zach Brown <zab@redhat.com>
---
 ctree.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ctree.h b/ctree.h
index 3c65d0a..bfe9f72 100644
--- a/ctree.h
+++ b/ctree.h
@@ -38,7 +38,7 @@
 struct btrfs_root;
 struct btrfs_trans_handle;
 struct btrfs_free_space_ctl;
-#define BTRFS_MAGIC 0x4D5F53665248425F /* ascii _BHRfS_M, no null */
+#define BTRFS_MAGIC 0x4D5F53665248425FULL /* ascii _BHRfS_M, no null */
 
 #define BTRFS_MAX_LEVEL 8
 
-- 
1.7.11.7

[PATCH 04/15] btrfs-progs: fix shadow symbols

[Zach Brown]

This fixes all the instances of warnings that symbols declared in blocks
shadow symbols with the same name in surrounding scopes:

 cmds-device.c:341:22: warning: symbol 'path' shadows an earlier one
 cmds-device.c:285:14: originally declared here

I just renamed or removed the risky shadow symbols instead of pulling
their blocks out into functions.

Signed-off-by: Zach Brown <zab@redhat.com>
---
 cmds-check.c   |  7 +++----
 cmds-device.c  | 10 +++++-----
 cmds-restore.c |  8 ++++----
 ctree.c        |  2 --
 volumes.c      |  2 +-
 5 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index 4e5f997..2318aed 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -3477,6 +3477,7 @@ static int run_next_block(struct btrfs_root *root,
 	u64 parent;
 	u64 owner;
 	u64 flags;
+	u64 ptr;
 	int ret;
 	int i;
 	int nritems;
@@ -3665,8 +3666,8 @@ static int run_next_block(struct btrfs_root *root,
 			btrfs_item_key_to_cpu(buf, &first_key, 0);
 		level = btrfs_header_level(buf);
 		for (i = 0; i < nritems; i++) {
-			u64 ptr = btrfs_node_blockptr(buf, i);
-			u32 size = btrfs_level_size(root, level - 1);
+			ptr = btrfs_node_blockptr(buf, i);
+			size = btrfs_level_size(root, level - 1);
 			btrfs_node_key_to_cpu(buf, &key, i);
 			ret = add_extent_rec(extent_cache, &key,
 					     ptr, size, 0, 0, 1, 0, 1, 0,
@@ -5317,8 +5318,6 @@ again:
 		ret = err;
 
 	if (trans) {
-		int err;
-
 		err = btrfs_commit_transaction(trans, root);
 		if (!ret)
 			ret = err;
diff --git a/cmds-device.c b/cmds-device.c
index 48ac526..06df864 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -282,7 +282,7 @@ static const char * const cmd_dev_stats_usage[] = {
 
 static int cmd_dev_stats(int argc, char **argv)
 {
-	char *path;
+	char *dev_path;
 	struct btrfs_ioctl_fs_info_args fi_args;
 	struct btrfs_ioctl_dev_info_args *di_args = NULL;
 	int ret;
@@ -314,16 +314,16 @@ static int cmd_dev_stats(int argc, char **argv)
 		return 1;
 	}
 
-	path = argv[optind];
+	dev_path = argv[optind];
 
-	fdmnt = open_path_or_dev_mnt(path, &dirstream);
+	fdmnt = open_path_or_dev_mnt(dev_path, &dirstream);
 
 	if (fdmnt < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+		fprintf(stderr, "ERROR: can't access '%s'\n", dev_path);
 		return 12;
 	}
 
-	ret = get_fs_info(path, &fi_args, &di_args);
+	ret = get_fs_info(dev_path, &fi_args, &di_args);
 	if (ret) {
 		fprintf(stderr, "ERROR: getting dev info for devstats failed: "
 				"%s\n", strerror(-ret));
diff --git a/cmds-restore.c b/cmds-restore.c
index 478c0b5..1e965a6 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -658,7 +658,7 @@ set_size:
 }
 
 static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
-		      const char *output_rootdir, const char *dir,
+		      const char *output_rootdir, const char *in_dir,
 		      const regex_t *mreg)
 {
 	struct btrfs_path *path;
@@ -716,7 +716,7 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 		if (loops++ >= 1024) {
 			printf("We have looped trying to restore files in %s "
 			       "too many times to be making progress, "
-			       "stopping\n", dir);
+			       "stopping\n", in_dir);
 			break;
 		}
 
@@ -764,7 +764,7 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 		btrfs_dir_item_key_to_cpu(leaf, dir_item, &location);
 
 		/* full path from root of btrfs being restored */
-		snprintf(fs_name, 4096, "%s/%s", dir, filename);
+		snprintf(fs_name, 4096, "%s/%s", in_dir, filename);
 
 		if (mreg && REG_NOMATCH == regexec(mreg, fs_name, 0, NULL, 0))
 			goto next;
@@ -896,7 +896,7 @@ next:
 	}
 
 	if (verbose)
-		printf("Done searching %s\n", dir);
+		printf("Done searching %s\n", in_dir);
 	btrfs_free_path(path);
 	return 0;
 }
diff --git a/ctree.c b/ctree.c
index a79ed13..1b093f6 100644
--- a/ctree.c
+++ b/ctree.c
@@ -2401,7 +2401,6 @@ int btrfs_insert_empty_items(struct btrfs_trans_handle *trans,
 	BUG_ON(slot < 0);
 
 	if (slot != nritems) {
-		int i;
 		unsigned int old_data = btrfs_item_end_nr(leaf, slot);
 
 		if (old_data < data_end) {
@@ -2580,7 +2579,6 @@ int btrfs_del_items(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	nritems = btrfs_header_nritems(leaf);
 
 	if (slot + nr != nritems) {
-		int i;
 		int data_end = leaf_data_end(root, leaf);
 
 		memmove_extent_buffer(leaf, btrfs_leaf_data(leaf) +
diff --git a/volumes.c b/volumes.c
index 1f172b5..e460bce 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1281,7 +1281,7 @@ again:
 				BTRFS_BLOCK_GROUP_RAID6)) {
 
 		if (raid_map) {
-			int i, rot;
+			int rot;
 			u64 tmp;
 			u64 raid56_full_stripe_start;
 			u64 full_stripe_len = nr_data_stripes(map) * map->stripe_len;
-- 
1.7.11.7

[PATCH 05/15] btrfs-progs: remove variable length stack arrays

[Zach Brown]

sparse hates variable length array definitions on the stack:

 btrfs-show-super.c:155:21: warning: Variable length array is used.

And it's right to.  They're a fragile construct that doesn't handle bad
input well at all.

Signed-off-by: Zach Brown <zab@redhat.com>
---
 btrfs-show-super.c |  2 +-
 volumes.c          | 16 ++++++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/btrfs-show-super.c b/btrfs-show-super.c
index c2e844d..0c3c73c 100644
--- a/btrfs-show-super.c
+++ b/btrfs-show-super.c
@@ -152,7 +152,7 @@ static int load_and_dump_sb(char *filename, int fd, u64 sb_bytenr)
 
 static int check_csum_sblock(void *sb, int csum_size)
 {
-	char result[csum_size];
+	char result[BTRFS_CSUM_SIZE];
 	u32 crc = ~(u32)0;
 
 	crc = btrfs_csum_data(NULL, (char *)sb + BTRFS_CSUM_SIZE,
diff --git a/volumes.c b/volumes.c
index e460bce..dba5b0e 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1779,12 +1779,15 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
 			     struct btrfs_multi_bio *multi,
 			     u64 stripe_len, u64 *raid_map)
 {
-	struct extent_buffer *ebs[multi->num_stripes], *p_eb = NULL, *q_eb = NULL;
+	struct extent_buffer **ebs, *p_eb = NULL, *q_eb = NULL;
 	int i;
 	int j;
 	int ret;
 	int alloc_size = eb->len;
 
+	ebs = kmalloc(sizeof(*ebs) * multi->num_stripes, GFP_NOFS);
+	BUG_ON(!ebs);
+
 	if (stripe_len > alloc_size)
 		alloc_size = stripe_len;
 
@@ -1813,7 +1816,12 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
 			q_eb = new_eb;
 	}
 	if (q_eb) {
-		void *pointers[multi->num_stripes];
+		void **pointers;
+
+		pointers = kmalloc(sizeof(*pointers) * multi->num_stripes,
+				   GFP_NOFS);
+		BUG_ON(!pointers);
+
 		ebs[multi->num_stripes - 2] = p_eb;
 		ebs[multi->num_stripes - 1] = q_eb;
 
@@ -1821,6 +1829,7 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
 			pointers[i] = ebs[i]->data;
 
 		raid6_gen_syndrome(multi->num_stripes, stripe_len, pointers);
+		kfree(pointers);
 	} else {
 		ebs[multi->num_stripes - 1] = p_eb;
 		memcpy(p_eb->data, ebs[0]->data, stripe_len);
@@ -1838,5 +1847,8 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
 		if (ebs[i] != eb)
 			kfree(ebs[i]);
 	}
+
+	kfree(ebs);
+
 	return 0;
 }
-- 
1.7.11.7

[PATCH 06/15] btrfs-print: define void function args

[Zach Brown]

This silences (reasonable) sparse warnings of the form:

	warning: non-ANSI function declaration of ..

Signed-off-by: Zach Brown <zab@redhat.com>
---
 btrfs-find-root.c | 2 +-
 btrfs-list.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/btrfs-find-root.c b/btrfs-find-root.c
index 1a7a80c..b48c800 100644
--- a/btrfs-find-root.c
+++ b/btrfs-find-root.c
@@ -40,7 +40,7 @@ static u64 search_objectid = BTRFS_ROOT_TREE_OBJECTID;
 static u64 search_generation = 0;
 static unsigned long search_level = 0;
 
-static void usage()
+static void usage(void)
 {
 	fprintf(stderr, "Usage: find-roots [-o search_objectid] "
 		"[ -g search_generation ] [ -l search_level ] <device>\n");
diff --git a/btrfs-list.c b/btrfs-list.c
index 214cb45..55508d2 100644
--- a/btrfs-list.c
+++ b/btrfs-list.c
@@ -1418,7 +1418,7 @@ static void print_single_volume_info_default(struct root_info *subv)
 	printf("\n");
 }
 
-static void print_all_volume_info_tab_head()
+static void print_all_volume_info_tab_head(void)
 {
 	int i;
 	int len;
-- 
1.7.11.7

[PATCH 07/15] btrfs-progs: fix endian bugs in chunk rebuilding

[Zach Brown]

A disk_key was set by hand instead of using the endian helpers.

I *think* the second one is just a typo.  The chunk's num_stripes was
already initialized from the record, but it's le16.  So we'll set the
item's size based on the record's native num_stripes.

Signed-off-by: Zach Brown <zab@redhat.com>
---
 cmds-chunk.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/cmds-chunk.c b/cmds-chunk.c
index 16f399e..54f0573 100644
--- a/cmds-chunk.c
+++ b/cmds-chunk.c
@@ -1031,9 +1031,9 @@ static int __rebuild_chunk_root(struct btrfs_trans_handle *trans,
 		if (min_devid > dev->devid)
 			min_devid = dev->devid;
 	}
-	disk_key.objectid = BTRFS_DEV_ITEMS_OBJECTID;
-	disk_key.type = BTRFS_DEV_ITEM_KEY;
-	disk_key.offset = min_devid;
+	btrfs_set_disk_key_objectid(&disk_key, BTRFS_DEV_ITEMS_OBJECTID);
+	btrfs_set_disk_key_type(&disk_key, BTRFS_DEV_ITEM_KEY);
+	btrfs_set_disk_key_offset(&disk_key, min_devid);
 
 	cow = btrfs_alloc_free_block(trans, root, root->sectorsize,
 				     BTRFS_CHUNK_TREE_OBJECTID,
@@ -1117,7 +1117,7 @@ static int __rebuild_chunk_items(struct btrfs_trans_handle *trans,
 		key.offset = chunk_rec->offset;
 
 		ret = btrfs_insert_item(trans, chunk_root, &key, chunk,
-				btrfs_chunk_item_size(chunk->num_stripes));
+				btrfs_chunk_item_size(chunk_rec->num_stripes));
 		free(chunk);
 		if (ret)
 			return ret;
-- 
1.7.11.7

Re: [PATCH 07/15] btrfs-progs: fix endian bugs in chunk rebuilding

[David Sterba]

On Wed, Aug 14, 2013 at 04:16:37PM -0700, Zach Brown wrote:
> I *think* the second one is just a typo.  The chunk's num_stripes was
> already initialized from the record, but it's le16.  So we'll set the
> item's size based on the record's native num_stripes.

> @@ -1117,7 +1117,7 @@ static int __rebuild_chunk_items(struct btrfs_trans_handle *trans,
>  		key.offset = chunk_rec->offset;
>  
>  		ret = btrfs_insert_item(trans, chunk_root, &key, chunk,
> -				btrfs_chunk_item_size(chunk->num_stripes));
> +				btrfs_chunk_item_size(chunk_rec->num_stripes));

AFAICS, this is an equivalent change, chunk gets copied from chunk_rec
via create_chunk_item a few lines above. It looks more consistent with
chunk_rec, though.

>  		free(chunk);
>  		if (ret)
>  			return ret;

[PATCH 08/15] btrfs-progs: fix extent key endian bug in repair

[Zach Brown]

Extents rebuilt from backrefs can have their objectid mangled.  The code
tried to build a disk_key by hand and got the swabbing backwards.

Signed-off-by: Zach Brown <zab@redhat.com>
---
 cmds-check.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/cmds-check.c b/cmds-check.c
index 2318aed..2d5162c 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -3931,9 +3931,12 @@ static int record_extent(struct btrfs_trans_handle *trans,
 			bi = (struct btrfs_tree_block_info *)(ei + 1);
 			memset_extent_buffer(leaf, 0, (unsigned long)bi,
 					     sizeof(*bi));
-			memset(&copy_key, 0, sizeof(copy_key));
 
-			copy_key.objectid = le64_to_cpu(rec->info_objectid);
+			btrfs_set_disk_key_objectid(&copy_key,
+						    rec->info_objectid);
+			btrfs_set_disk_key_type(&copy_key, 0);
+			btrfs_set_disk_key_offset(&copy_key, 0);
+
 			btrfs_set_tree_block_level(leaf, bi, rec->info_level);
 			btrfs_set_tree_block_key(leaf, bi, &copy_key);
 
-- 
1.7.11.7

[PATCH 09/15] btrfs-progs: fix in-place byte swapping

[Zach Brown]

Storing fixed-endian values in native cpu types defeats the purpose of
using sparse endian types to find endian conversion bugs.

Signed-off-by: Zach Brown <zab@redhat.com>
---
 print-tree.c | 6 +++---
 uuid-tree.c  | 5 +++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/print-tree.c b/print-tree.c
index 761448d..913b1bb 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -688,11 +688,11 @@ static void print_uuid_item(struct extent_buffer *l, unsigned long offset,
 		return;
 	}
 	while (item_size) {
-		u64 subvol_id;
+		__le64 subvol_id;
 
 		read_extent_buffer(l, &subvol_id, offset, sizeof(u64));
-		subvol_id = le64_to_cpu(subvol_id);
-		printf("\t\tsubvol_id %llu\n", (unsigned long long)subvol_id);
+		printf("\t\tsubvol_id %llu\n",
+			(unsigned long long)le64_to_cpu(subvol_id));
 		item_size -= sizeof(u64);
 		offset += sizeof(u64);
 	}
diff --git a/uuid-tree.c b/uuid-tree.c
index 3a4c48e..39c3f3f 100644
--- a/uuid-tree.c
+++ b/uuid-tree.c
@@ -43,6 +43,7 @@ static int btrfs_uuid_tree_lookup_any(int fd, const u8 *uuid, u8 type,
 	struct btrfs_ioctl_search_args search_arg;
 	struct btrfs_ioctl_search_header *search_header;
 	u32 item_size;
+	__le64 lesubid;
 
 	btrfs_uuid_to_key(uuid, &key_objectid, &key_offset);
 
@@ -82,8 +83,8 @@ static int btrfs_uuid_tree_lookup_any(int fd, const u8 *uuid, u8 type,
 	}
 
 	/* return first stored id */
-	memcpy(subid, search_header + 1, sizeof(*subid));
-	*subid = le64_to_cpu(*subid);
+	memcpy(&lesubid, search_header + 1, sizeof(lesubid));
+	*subid = le64_to_cpu(lesubid);
 
 out:
 	return ret;
-- 
1.7.11.7

[PATCH 10/15] btrfs-progs: fix qgroup realloc inheritance

[Zach Brown]

qgroup.c:82:23: warning: memcpy with byte count of 0
qgroup.c:83:23: warning: memcpy with byte count of 0

The inheritance wasn't copying qgroups[] because a confused sizeof()
gave 0 byte memcpy()s.  It's been like this for the year since it was
merged, so I guess this isn't a very important thing to do :).

Signed-off-by: Zach Brown <zab@redhat.com>
---
 qgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qgroup.c b/qgroup.c
index 038c4dc..86fe2b2 100644
--- a/qgroup.c
+++ b/qgroup.c
@@ -74,7 +74,7 @@ qgroup_inherit_realloc(struct btrfs_qgroup_inherit **inherit, int n, int pos)
 
 	if (*inherit) {
 		struct btrfs_qgroup_inherit *i = *inherit;
-		int s = sizeof(out->qgroups);
+		int s = sizeof(out->qgroups[0]);
 
 		out->num_qgroups = i->num_qgroups;
 		out->num_ref_copies = i->num_ref_copies;
-- 
1.7.11.7

Re: [PATCH 10/15] btrfs-progs: fix qgroup realloc inheritance

[Arne Jansen]

On 08/15/13 01:16, Zach Brown wrote:
> qgroup.c:82:23: warning: memcpy with byte count of 0
> qgroup.c:83:23: warning: memcpy with byte count of 0
> 
> The inheritance wasn't copying qgroups[] because a confused sizeof()
> gave 0 byte memcpy()s.  It's been like this for the year since it was
> merged, so I guess this isn't a very important thing to do :).

It only seems to hit if you give -[cx] before -i. I guess only very
few people use these options in the first place. They are primarily
for hosting providers.

Reviewed-by: Arne Jansen <sensille@gmx.net>
> 
> Signed-off-by: Zach Brown <zab@redhat.com>
> ---
>  qgroup.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qgroup.c b/qgroup.c
> index 038c4dc..86fe2b2 100644
> --- a/qgroup.c
> +++ b/qgroup.c
> @@ -74,7 +74,7 @@ qgroup_inherit_realloc(struct btrfs_qgroup_inherit **inherit, int n, int pos)
>  
>  	if (*inherit) {
>  		struct btrfs_qgroup_inherit *i = *inherit;
> -		int s = sizeof(out->qgroups);
> +		int s = sizeof(out->qgroups[0]);
>  
>  		out->num_qgroups = i->num_qgroups;
>  		out->num_ref_copies = i->num_ref_copies;
> 

[PATCH 11/15] btrfs-progs: make many private symbols static

[Zach Brown]

Signed-off-by: Zach Brown <zab@redhat.com>
---
 btrfs-convert.c    | 2 +-
 btrfs.c            | 6 +++---
 cmds-dedup.c       | 2 +-
 crc32c.c           | 4 ++--
 ctree.c            | 4 ++--
 free-space-cache.c | 4 ++--
 help.c             | 2 +-
 radix-tree.c       | 2 +-
 8 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/btrfs-convert.c b/btrfs-convert.c
index 9a7da57..828f361 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -223,7 +223,7 @@ static int custom_free_extent(struct btrfs_root *root, u64 bytenr,
 	return intersect_with_sb(bytenr, num_bytes);
 }
 
-struct btrfs_extent_ops extent_ops = {
+static struct btrfs_extent_ops extent_ops = {
 	.alloc_extent = custom_alloc_extent,
 	.free_extent = custom_free_extent,
 };
diff --git a/btrfs.c b/btrfs.c
index 3e53a6a..050c9c3 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -31,7 +31,7 @@ static const char * const btrfs_cmd_group_usage[] = {
 static const char btrfs_cmd_group_info[] =
 	"Use --help as an argument for information on a specific group or command.";
 
-char argv0_buf[ARGV0_BUF_SIZE] = "btrfs";
+static char argv0_buf[ARGV0_BUF_SIZE] = "btrfs";
 
 static inline const char *skip_prefix(const char *str, const char *prefix)
 {
@@ -184,7 +184,7 @@ int check_argc_max(int nargs, int expected)
 	return 0;
 }
 
-const struct cmd_group btrfs_cmd_group;
+static const struct cmd_group btrfs_cmd_group;
 
 static const char * const cmd_help_usage[] = {
 	"btrfs help [--full]",
@@ -239,7 +239,7 @@ static int handle_options(int *argc, char ***argv)
 	return (*argv) - orig_argv;
 }
 
-const struct cmd_group btrfs_cmd_group = {
+static const struct cmd_group btrfs_cmd_group = {
 	btrfs_cmd_group_usage, btrfs_cmd_group_info, {
 		{ "subvolume", cmd_subvolume, NULL, &subvolume_cmd_group, 0 },
 		{ "filesystem", cmd_filesystem, NULL, &filesystem_cmd_group, 0 },
diff --git a/cmds-dedup.c b/cmds-dedup.c
index 215fddc..a22b5c9 100644
--- a/cmds-dedup.c
+++ b/cmds-dedup.c
@@ -30,7 +30,7 @@ static const char * const dedup_cmd_group_usage[] = {
 	NULL
 };
 
-int dedup_ctl(int cmd, int argc, char **argv)
+static int dedup_ctl(int cmd, int argc, char **argv)
 {
 	int ret = 0;
 	int fd;
diff --git a/crc32c.c b/crc32c.c
index c285d6e..dfa4e6c 100644
--- a/crc32c.c
+++ b/crc32c.c
@@ -62,7 +62,7 @@ static uint32_t crc32c_intel_le_hw_byte(uint32_t crc, unsigned char const *data,
  * Steps through buffer one byte at at time, calculates reflected 
  * crc using table.
  */
-uint32_t crc32c_intel(u32 crc, unsigned char const *data, unsigned long length)
+static uint32_t crc32c_intel(u32 crc, unsigned char const *data, unsigned long length)
 {
 	unsigned int iquotient = length / SCALE_F;
 	unsigned int iremainder = length % SCALE_F;
@@ -100,7 +100,7 @@ static void do_cpuid(unsigned int *eax, unsigned int *ebx, unsigned int *ecx,
 		: "eax", "ebx", "ecx", "edx");
 }
 
-void crc32c_intel_probe(void)
+static void crc32c_intel_probe(void)
 {
 	if (!crc32c_probed) {
 		unsigned int eax, ebx, ecx, edx;
diff --git a/ctree.c b/ctree.c
index 1b093f6..8b1fc07 100644
--- a/ctree.c
+++ b/ctree.c
@@ -136,8 +136,8 @@ int btrfs_copy_root(struct btrfs_trans_handle *trans,
 /*
  * check if the tree block can be shared by multiple trees
  */
-int btrfs_block_can_be_shared(struct btrfs_root *root,
-			      struct extent_buffer *buf)
+static int btrfs_block_can_be_shared(struct btrfs_root *root,
+			             struct extent_buffer *buf)
 {
 	/*
 	 * Tree blocks not in refernece counted trees and tree roots
diff --git a/free-space-cache.c b/free-space-cache.c
index 35edac2..ddeeeb6 100644
--- a/free-space-cache.c
+++ b/free-space-cache.c
@@ -792,8 +792,8 @@ void btrfs_remove_free_space_cache(struct btrfs_block_group_cache *block_group)
 	__btrfs_remove_free_space_cache(block_group->free_space_ctl);
 }
 
-int btrfs_add_free_space(struct btrfs_free_space_ctl *ctl, u64 offset,
-			 u64 bytes)
+static int btrfs_add_free_space(struct btrfs_free_space_ctl *ctl, u64 offset,
+				u64 bytes)
 {
 	struct btrfs_free_space *info;
 	int ret = 0;
diff --git a/help.c b/help.c
index 6d04293..d429a6b 100644
--- a/help.c
+++ b/help.c
@@ -20,7 +20,7 @@
 
 #include "commands.h"
 
-extern char argv0_buf[ARGV0_BUF_SIZE];
+static char argv0_buf[ARGV0_BUF_SIZE];
 
 #define USAGE_SHORT		1U
 #define USAGE_LONG		2U
diff --git a/radix-tree.c b/radix-tree.c
index ed01810..4f295fc 100644
--- a/radix-tree.c
+++ b/radix-tree.c
@@ -73,7 +73,7 @@ struct radix_tree_preload {
 	int nr;
 	struct radix_tree_node *nodes[RADIX_TREE_MAX_PATH];
 };
-struct radix_tree_preload radix_tree_preloads = { 0, };
+static struct radix_tree_preload radix_tree_preloads = { 0, };
 
 static inline gfp_t root_gfp_mask(struct radix_tree_root *root)
 {
-- 
1.7.11.7

[PATCH 12/15] btrfs-progs: fix unaligned compat endian warnings

[Zach Brown]

The _una_ struct's entire job is to pass an argument to le*_to_cpu.  So
it's a little embarassing that it uses a native cpu types and generates
endian warnings.

ctree.h:1616:1: warning: incorrect type in assignment (different base types)
ctree.h:1616:1:    expected unsigned long long [unsigned] [usertype] x
ctree.h:1616:1:    got restricted __le64 [usertype] <noident>

Signed-off-by: Zach Brown <zab@redhat.com>
---
 kerncompat.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kerncompat.h b/kerncompat.h
index 4d78288..1fc2b34 100644
--- a/kerncompat.h
+++ b/kerncompat.h
@@ -277,9 +277,9 @@ typedef u64 __bitwise __be64;
 #define le16_to_cpu(x) ((__force u16)(__le16)(x))
 #endif
 
-struct __una_u16 { u16 x; } __attribute__((__packed__));
-struct __una_u32 { u32 x; } __attribute__((__packed__));
-struct __una_u64 { u64 x; } __attribute__((__packed__));
+struct __una_u16 { __le16 x; } __attribute__((__packed__));
+struct __una_u32 { __le32 x; } __attribute__((__packed__));
+struct __una_u64 { __le64 x; } __attribute__((__packed__));
 
 #define get_unaligned_le8(p) (*((u8 *)(p)))
 #define put_unaligned_le8(val,p) ((*((u8 *)(p))) = (val))
-- 
1.7.11.7

[PATCH 13/15] btrfs-progs: don't use <linux/fs.h>

[Zach Brown]

sparse can freak out when <linux/fs.h> is included because it redefines
approximately a gazillion symbols already found in <sys/mount.h>:

/usr/include/linux/fs.h:203:9: warning: preprocessor token MS_RDONLY redefined
/usr/include/sys/mount.h:37:9: this was the original definition

Happily, we don't actually need to include the low-level <linux/fs.h>
for anything.  One assumes it was just carried over from kernel space.

Signed-off-by: Zach Brown <zab@redhat.com>
---
 btrfs-convert.c | 1 -
 mkfs.c          | 1 -
 2 files changed, 2 deletions(-)

diff --git a/btrfs-convert.c b/btrfs-convert.c
index 828f361..a1fb645 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -31,7 +31,6 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <uuid/uuid.h>
-#include <linux/fs.h>
 
 #include "ctree.h"
 #include "disk-io.h"
diff --git a/mkfs.c b/mkfs.c
index 5724dec..8e38db7 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -33,7 +33,6 @@
 #include <unistd.h>
 #include <getopt.h>
 #include <uuid/uuid.h>
-#include <linux/fs.h>
 #include <ctype.h>
 #include <attr/xattr.h>
 #include <blkid/blkid.h>
-- 
1.7.11.7

[PATCH 14/15] btrfs-progs: give raid6.c its exported prototypes

[Zach Brown]

raid6.c is built without access to the prototypes of functions it
exports.

  warning: symbol 'raid6_gen_syndrome' was not declared.  Should it be static?

They could be changed and get out of sync of the exported prototypes
without errors.   So we add disk-io.h, and its dependency ctree.h, so
that it has a chance to check that its exported prototypes are correct.

Signed-off-by: Zach Brown <zab@redhat.com>
---
 raid6.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/raid6.c b/raid6.c
index ce0f655..a6ee483 100644
--- a/raid6.c
+++ b/raid6.c
@@ -20,6 +20,8 @@
 #include <stdint.h>
 #include <unistd.h>
 #include "kerncompat.h"
+#include "ctree.h"
+#include "disk-io.h"
 
 /*
  * This is the C data type to use
-- 
1.7.11.7

[PATCH 15/15] btrfs-progs: use NULL instead of 0

[Zach Brown]

These were mostly in option structs but there were a few gross string
pointer arguments given as 0.

Signed-off-by: Zach Brown <zab@redhat.com>
---
 btrfs-map-logical.c |  2 +-
 btrfs.c             |  2 +-
 cmds-balance.c      |  6 +++---
 cmds-check.c        |  2 +-
 cmds-dedup.c        |  4 ++--
 cmds-device.c       |  2 +-
 cmds-filesystem.c   |  4 ++--
 cmds-inspect.c      |  2 +-
 cmds-qgroup.c       | 21 +++++++++++++--------
 cmds-quota.c        |  5 +++--
 cmds-replace.c      |  2 +-
 cmds-restore.c      |  2 +-
 cmds-scrub.c        |  2 +-
 cmds-subvolume.c    |  8 ++++----
 commands.h          |  2 ++
 mkfs.c              |  2 +-
 16 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/btrfs-map-logical.c b/btrfs-map-logical.c
index ee8407f..ea0a1ba 100644
--- a/btrfs-map-logical.c
+++ b/btrfs-map-logical.c
@@ -106,7 +106,7 @@ static struct option long_options[] = {
 	{ "copy", 1, NULL, 'c' },
 	{ "output", 1, NULL, 'o' },
 	{ "bytes", 1, NULL, 'b' },
-	{ 0, 0, 0, 0}
+	{ NULL, 0, NULL, 0}
 };
 
 int main(int ac, char **av)
diff --git a/btrfs.c b/btrfs.c
index 050c9c3..fc9429d 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -258,7 +258,7 @@ static const struct cmd_group btrfs_cmd_group = {
 		{ "dedup", cmd_dedup, NULL, &dedup_cmd_group, 0 },
 		{ "help", cmd_help, cmd_help_usage, NULL, 0 },
 		{ "version", cmd_version, cmd_version_usage, NULL, 0 },
-		{ 0, 0, 0, 0, 0 }
+		NULL_CMD_STRUCT,
 	},
 };
 
diff --git a/cmds-balance.c b/cmds-balance.c
index c78b726..a512368 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -382,7 +382,7 @@ static int cmd_balance_start(int argc, char **argv)
 			{ "system", optional_argument, NULL, 's' },
 			{ "force", no_argument, NULL, 'f' },
 			{ "verbose", no_argument, NULL, 'v' },
-			{ 0, 0, 0, 0 }
+			{ NULL, no_argument, NULL, 0 },
 		};
 
 		int opt = getopt_long(argc, argv, "d::s::m::fv", longopts,
@@ -641,7 +641,7 @@ static int cmd_balance_status(int argc, char **argv)
 		int longindex;
 		static struct option longopts[] = {
 			{ "verbose", no_argument, NULL, 'v' },
-			{ 0, 0, 0, 0}
+			{ NULL, no_argument, NULL, 0}
 		};
 
 		int opt = getopt_long(argc, argv, "v", longopts, &longindex);
@@ -713,7 +713,7 @@ const struct cmd_group balance_cmd_group = {
 		{ "cancel", cmd_balance_cancel, cmd_balance_cancel_usage, NULL, 0 },
 		{ "resume", cmd_balance_resume, cmd_balance_resume_usage, NULL, 0 },
 		{ "status", cmd_balance_status, cmd_balance_status_usage, NULL, 0 },
-		{ 0, 0, 0, 0, 0 }
+		NULL_CMD_STRUCT,
 	}
 };
 
diff --git a/cmds-check.c b/cmds-check.c
index 2d5162c..df18c43 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -5739,7 +5739,7 @@ static struct option long_options[] = {
 	{ "repair", 0, NULL, 0 },
 	{ "init-csum-tree", 0, NULL, 0 },
 	{ "init-extent-tree", 0, NULL, 0 },
-	{ 0, 0, 0, 0}
+	{ NULL, 0, NULL, 0}
 };
 
 const char * const cmd_check_usage[] = {
diff --git a/cmds-dedup.c b/cmds-dedup.c
index a22b5c9..7d96673 100644
--- a/cmds-dedup.c
+++ b/cmds-dedup.c
@@ -91,8 +91,8 @@ static int cmd_dedup_unreg(int argc, char **argv)
 const struct cmd_group dedup_cmd_group = {
 	dedup_cmd_group_usage, NULL, {
 		{ "register", cmd_dedup_reg, cmd_dedup_reg_usage, NULL, 0 },
-		{ "unregister", cmd_dedup_unreg, cmd_dedup_unreg_usage, 0, 0 },
-		{ 0, 0, 0, 0, 0 }
+		{ "unregister", cmd_dedup_unreg, cmd_dedup_unreg_usage, NULL, 0 },
+		NULL_CMD_STRUCT,
 	}
 };
 
diff --git a/cmds-device.c b/cmds-device.c
index 06df864..764e5ab 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -396,7 +396,7 @@ const struct cmd_group device_cmd_group = {
 		{ "scan", cmd_scan_dev, cmd_scan_dev_usage, NULL, 0 },
 		{ "ready", cmd_ready_dev, cmd_ready_dev_usage, NULL, 0 },
 		{ "stats", cmd_dev_stats, cmd_dev_stats_usage, NULL, 0 },
-		{ 0, 0, 0, 0, 0 }
+		NULL_CMD_STRUCT,
 	}
 };
 
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index a4e30ea..f545eb2 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -231,7 +231,7 @@ static int cmd_show(int argc, char **argv)
 	struct list_head *all_uuids;
 	struct btrfs_fs_devices *fs_devices;
 	struct list_head *cur_uuid;
-	char *search = 0;
+	char *search = NULL;
 	int ret;
 	int where = BTRFS_SCAN_PROC;
 	int searchstart = 1;
@@ -512,7 +512,7 @@ const struct cmd_group filesystem_cmd_group = {
 		{ "balance", cmd_balance, NULL, &balance_cmd_group, 1 },
 		{ "resize", cmd_resize, cmd_resize_usage, NULL, 0 },
 		{ "label", cmd_label, cmd_label_usage, NULL, 0 },
-		{ 0, 0, 0, 0, 0 },
+		NULL_CMD_STRUCT,
 	}
 };
 
diff --git a/cmds-inspect.c b/cmds-inspect.c
index 8417e66..341ebd3 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -309,7 +309,7 @@ const struct cmd_group inspect_cmd_group = {
 			cmd_logical_resolve_usage, NULL, 0 },
 		{ "subvolid-resolve", cmd_subvolid_resolve,
 			cmd_subvolid_resolve_usage, NULL, 0 },
-		{ 0, 0, 0, 0, 0 }
+		NULL_CMD_STRUCT,
 	}
 };
 
diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 891d46c..8da18eb 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -428,14 +428,19 @@ static int cmd_qgroup_limit(int argc, char **argv)
 
 const struct cmd_group qgroup_cmd_group = {
 	qgroup_cmd_group_usage, NULL, {
-		{ "assign", cmd_qgroup_assign, cmd_qgroup_assign_usage, 0, 0 },
-		{ "remove", cmd_qgroup_remove, cmd_qgroup_remove_usage, 0, 0 },
-		{ "create", cmd_qgroup_create, cmd_qgroup_create_usage, 0, 0 },
-		{ "destroy", cmd_qgroup_destroy,
-		  cmd_qgroup_destroy_usage, 0, 0 },
-		{ "show", cmd_qgroup_show, cmd_qgroup_show_usage, 0, 0 },
-		{ "limit", cmd_qgroup_limit, cmd_qgroup_limit_usage, 0, 0 },
-		{ 0, 0, 0, 0, 0 }
+		{ "assign", cmd_qgroup_assign, cmd_qgroup_assign_usage,
+		   NULL, 0 },
+		{ "remove", cmd_qgroup_remove, cmd_qgroup_remove_usage,
+		   NULL, 0 },
+		{ "create", cmd_qgroup_create, cmd_qgroup_create_usage,
+		   NULL, 0 },
+		{ "destroy", cmd_qgroup_destroy, cmd_qgroup_destroy_usage,
+		   NULL, 0 },
+		{ "show", cmd_qgroup_show, cmd_qgroup_show_usage,
+		   NULL, 0 },
+		{ "limit", cmd_qgroup_limit, cmd_qgroup_limit_usage,
+		   NULL, 0 },
+		NULL_CMD_STRUCT,
 	}
 };
 
diff --git a/cmds-quota.c b/cmds-quota.c
index 3bdd5f6..5317f72 100644
--- a/cmds-quota.c
+++ b/cmds-quota.c
@@ -179,9 +179,10 @@ static int cmd_quota_rescan(int argc, char **argv)
 const struct cmd_group quota_cmd_group = {
 	quota_cmd_group_usage, NULL, {
 		{ "enable", cmd_quota_enable, cmd_quota_enable_usage, NULL, 0 },
-		{ "disable", cmd_quota_disable, cmd_quota_disable_usage, 0, 0 },
+		{ "disable", cmd_quota_disable, cmd_quota_disable_usage,
+		   NULL, 0 },
 		{ "rescan", cmd_quota_rescan, cmd_quota_rescan_usage, NULL, 0 },
-		{ 0, 0, 0, 0, 0 }
+		NULL_CMD_STRUCT,
 	}
 };
 
diff --git a/cmds-replace.c b/cmds-replace.c
index 8ed92c4..b28bd3d 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -555,7 +555,7 @@ const struct cmd_group replace_cmd_group = {
 		  0 },
 		{ "cancel", cmd_cancel_replace, cmd_cancel_replace_usage, NULL,
 		  0 },
-		{ 0, 0, 0, 0, 0 }
+		NULL_CMD_STRUCT,
 	}
 };
 
diff --git a/cmds-restore.c b/cmds-restore.c
index 1e965a6..4c87483 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -1086,7 +1086,7 @@ out:
 
 static struct option long_options[] = {
 	{ "path-regex", 1, NULL, 256},
-	{ 0, 0, 0, 0}
+	{ NULL, 0, NULL, 0}
 };
 
 const char * const cmd_restore_usage[] = {
diff --git a/cmds-scrub.c b/cmds-scrub.c
index 08810b6..56f269f 100644
--- a/cmds-scrub.c
+++ b/cmds-scrub.c
@@ -1736,7 +1736,7 @@ const struct cmd_group scrub_cmd_group = {
 		{ "cancel", cmd_scrub_cancel, cmd_scrub_cancel_usage, NULL, 0 },
 		{ "resume", cmd_scrub_resume, cmd_scrub_resume_usage, NULL, 0 },
 		{ "status", cmd_scrub_status, cmd_scrub_status_usage, NULL, 0 },
-		{ 0, 0, 0, 0, 0 }
+		NULL_CMD_STRUCT,
 	}
 };
 
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index b1e5361..c62930c 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -229,7 +229,7 @@ again:
 		goto out;
 	}
 
-	cpath = realpath(path, 0);
+	cpath = realpath(path, NULL);
 	dname = strdup(cpath);
 	dname = dirname(dname);
 	vname = strdup(cpath);
@@ -331,7 +331,7 @@ static int cmd_subvol_list(int argc, char **argv)
 	int is_only_in_path = 0;
 	struct option long_options[] = {
 		{"sort", 1, NULL, 'S'},
-		{0, 0, 0, 0}
+		{NULL, 0, NULL, 0}
 	};
 	DIR *dirstream = NULL;
 
@@ -806,7 +806,7 @@ static int cmd_subvol_show(int argc, char **argv)
 	if (check_argc_exact(argc, 2))
 		usage(cmd_subvol_show_usage);
 
-	fullpath = realpath(argv[1], 0);
+	fullpath = realpath(argv[1], NULL);
 	if (!fullpath) {
 		fprintf(stderr, "ERROR: finding real path for '%s', %s\n",
 			argv[1], strerror(errno));
@@ -951,7 +951,7 @@ const struct cmd_group subvolume_cmd_group = {
 			cmd_subvol_set_default_usage, NULL, 0 },
 		{ "find-new", cmd_find_new, cmd_find_new_usage, NULL, 0 },
 		{ "show", cmd_subvol_show, cmd_subvol_show_usage, NULL, 0 },
-		{ 0, 0, 0, 0, 0 }
+		NULL_CMD_STRUCT,
 	}
 };
 
diff --git a/commands.h b/commands.h
index ac7dd23..65d5fea 100644
--- a/commands.h
+++ b/commands.h
@@ -50,6 +50,8 @@ struct cmd_struct {
 	int hidden;
 };
 
+#define NULL_CMD_STRUCT {NULL, NULL, NULL, NULL, 0}
+
 struct cmd_group {
 	const char * const *usagestr;
 	const char *infostr;
diff --git a/mkfs.c b/mkfs.c
index 8e38db7..cac3a84 100644
--- a/mkfs.c
+++ b/mkfs.c
@@ -326,7 +326,7 @@ static struct option long_options[] = {
 	{ "rootdir", 1, NULL, 'r' },
 	{ "nodiscard", 0, NULL, 'K' },
 	{ "features", 0, NULL, 'O' },
-	{ 0, 0, 0, 0}
+	{ NULL, 0, NULL, 0}
 };
 
 static int add_directory_items(struct btrfs_trans_handle *trans,
-- 
1.7.11.7

Re: [PATCH 15/15] btrfs-progs: use NULL instead of 0

[David Sterba]

On Wed, Aug 14, 2013 at 04:16:45PM -0700, Zach Brown wrote:
> +	{ NULL, 0, NULL, 0}

> +		NULL_CMD_STRUCT,

It's always the last item, so I think the comma should go away,
currently it's mixed. I'll fix it during commit, too trivial to bother
you.

david

Re: [PATCH 15/15] btrfs-progs: use NULL instead of 0

[Zach Brown]

On Fri, Aug 30, 2013 at 06:25:54PM +0200, David Sterba wrote:
> On Wed, Aug 14, 2013 at 04:16:45PM -0700, Zach Brown wrote:
> > +	{ NULL, 0, NULL, 0}
> 
> > +		NULL_CMD_STRUCT,
> 
> It's always the last item, so I think the comma should go away,
> currently it's mixed. I'll fix it during commit, too trivial to bother
> you.

Agreed, I'm cool with whatever you prefer.

Thanks for merging these!

- z

Re: [RFC] btrfs-progs: fix sparse checking and warnings

[David Sterba]

I went through the actual fixes and they're selfcontained and now in
integration. Actually, the compilation fail in 1/1 might be caused by a
old sparse utility on my box, I'll check again later.

thanks,
david