[PATCH 0/6] Changes to fssum to support POSIX

[PATCH 0/6] Changes to fssum to support POSIX

[Arvind Raghavan]

This patch series modifies fssum to be able to verify POSIX
fsync/fdatasync guarantees. In particular, it allows single
files/symlinks to be passed as input, which allows us to verify
an fsync on a single file without checking the entire directory.

It also adds an option for non-recursive directory traversal,
which is useful because POSIX fsync doesn't guarantee that all
subdirectories are synced. Finally, it adds a flag that allows
file size to be ignored (currently enabled by default). This is
useful because POSIX fsync of a directory doesn't guarantee that
the data in the children is persisted.

These changes will be followed up with several Crashmonkey [1]
tests which make use of these changes.

[1] https://github.com/utsaslab/crashmonkey

Arvind Raghavan (6):
  src/fssum: Make sum_file_data global
  src/fssum: Refactor recursive traversal
  src/fssum: Add flag -R for non-recursive mode
  src/fssum: Add a flag for including file size in checksum
  src/fssum: Allow single file input
  src/fssum: Fix whitespace in usage

 src/fssum.c | 364 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 213 insertions(+), 151 deletions(-)

-- 
2.20.1

[PATCH 1/6] src/fssum: Make sum_file_data global

[Arvind Raghavan]

Makes deciding which function to use for summing file data happen once
in main instead of for each file.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
---
Moved into separate patch as per Amir's suggestion.

 src/fssum.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 3d97a70b..30f456c2 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -62,6 +62,7 @@ int n_excludes = 0;
 int verbose = 0;
 FILE *out_fp;
 FILE *in_fp;
+sum_file_data_t sum_file_data;
 
 enum _flags {
 	FLAG_UID,
@@ -514,8 +515,6 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 	int ret;
 	int fd;
 	int excl;
-	sum_file_data_t sum_file_data = flags[FLAG_STRUCTURE] ?
-			sum_file_data_strict : sum_file_data_permissive;
 	struct stat64 dir_st;
 
 	if (fstat64(dirfd, &dir_st)) {
@@ -824,6 +823,9 @@ main(int argc, char *argv[])
 			flagstring[i] -= 'a' - 'A';
 	}
 
+	sum_file_data = flags[FLAG_STRUCTURE] ?
+			sum_file_data_strict : sum_file_data_permissive;
+
 	path = argv[optind];
 	plen = strlen(path);
 	if (path[plen - 1] == '/') {
-- 
2.20.1

[PATCH 2/6] src/fssum: Refactor recursive traversal

[Arvind Raghavan]

Moves some logic from the recursive directory traversal into a helper
function to make it easier to add support for regular files. Does not
change functionality.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
---
Includes change to fstatat instead of fchdir and lstat.

 src/fssum.c | 265 ++++++++++++++++++++++++++++------------------------
 1 file changed, 141 insertions(+), 124 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 30f456c2..81e5cf5d 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -503,6 +503,10 @@ malformed:
 		excess_file(fn);
 }
 
+void
+sum_one(int dirfd, int level, sum_t *dircs, char *path_prefix,
+		char *path_in, char *name);
+
 void
 sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 {
@@ -513,8 +517,6 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 	int entries = 0;
 	int i;
 	int ret;
-	int fd;
-	int excl;
 	struct stat64 dir_st;
 
 	if (fstat64(dirfd, &dir_st)) {
@@ -549,152 +551,167 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
 	qsort(namelist, entries, sizeof(*namelist), namecmp);
 	for (i = 0; i < entries; ++i) {
 		struct stat64 st;
-		sum_t cs;
-		sum_t meta;
-		char *path;
-
-		sum_init(&cs);
-		sum_init(&meta);
-		path = alloc(strlen(path_in) + strlen(namelist[i]) + 3);
-		sprintf(path, "%s/%s", path_in, namelist[i]);
-		for (excl = 0; excl < n_excludes; ++excl) {
-			if (strncmp(excludes[excl].path, path,
-			    excludes[excl].len) == 0)
-				goto next;
-		}
 
-		ret = fchdir(dirfd);
-		if (ret == -1) {
-			perror("fchdir");
-			exit(-1);
-		}
-		ret = lstat64(namelist[i], &st);
+		ret = fstatat64(dirfd, namelist[i], &st, AT_SYMLINK_NOFOLLOW);
 		if (ret) {
-			fprintf(stderr, "stat failed for %s/%s: %s\n",
-				path_prefix, path, strerror(errno));
+			fprintf(stderr, "stat failed for %s/%s/%s: %s\n",
+				path_prefix, path_in, namelist[i],
+				strerror(errno));
 			exit(-1);
 		}
 
 		/* We are crossing into a different subvol, skip this subtree. */
 		if (st.st_dev != dir_st.st_dev)
-			goto next;
-
-		sum_add_u64(&meta, level);
-		sum_add(&meta, namelist[i], strlen(namelist[i]));
-		if (!S_ISDIR(st.st_mode))
-			sum_add_u64(&meta, st.st_nlink);
-		if (flags[FLAG_UID])
-			sum_add_u64(&meta, st.st_uid);
-		if (flags[FLAG_GID])
-			sum_add_u64(&meta, st.st_gid);
-		if (flags[FLAG_MODE])
-			sum_add_u64(&meta, st.st_mode);
-		if (flags[FLAG_ATIME])
-			sum_add_time(&meta, st.st_atime);
-		if (flags[FLAG_MTIME])
-			sum_add_time(&meta, st.st_mtime);
-		if (flags[FLAG_CTIME])
-			sum_add_time(&meta, st.st_ctime);
-		if (flags[FLAG_XATTRS] &&
-		    (S_ISDIR(st.st_mode) || S_ISREG(st.st_mode))) {
-			fd = openat(dirfd, namelist[i], 0);
-			if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
-				sum_add_u64(&meta, errno);
-			} else if (fd == -1) {
-				fprintf(stderr, "open failed for %s/%s: %s\n",
-					path_prefix, path, strerror(errno));
+			continue;
+
+		sum_one(dirfd, level, dircs, path_prefix, path_in, namelist[i]);
+	}
+}
+
+void
+sum_one(int dirfd, int level, sum_t *dircs, char *path_prefix,
+		char *path_in, char *name) {
+	sum_t cs;
+	sum_t meta;
+	int fd;
+	int ret;
+	int excl;
+	char* path;
+	struct stat64 st;
+
+	sum_init(&cs);
+	sum_init(&meta);
+	path = alloc(strlen(path_in) + strlen(name) + 3);
+	sprintf(path, "%s/%s", path_in, name);
+	for (excl = 0; excl < n_excludes; ++excl) {
+		if (strncmp(excludes[excl].path, path,
+			excludes[excl].len) == 0)
+			goto out;
+	}
+
+	ret = fstatat64(dirfd, name, &st, AT_SYMLINK_NOFOLLOW);
+	if (ret) {
+		fprintf(stderr, "stat failed for %s/%s: %s\n",
+			path_prefix, path, strerror(errno));
+		exit(-1);
+	}
+
+	sum_add_u64(&meta, level);
+	sum_add(&meta, name, strlen(name));
+	if (!S_ISDIR(st.st_mode))
+		sum_add_u64(&meta, st.st_nlink);
+	if (flags[FLAG_UID])
+		sum_add_u64(&meta, st.st_uid);
+	if (flags[FLAG_GID])
+		sum_add_u64(&meta, st.st_gid);
+	if (flags[FLAG_MODE])
+		sum_add_u64(&meta, st.st_mode);
+	if (flags[FLAG_ATIME])
+		sum_add_time(&meta, st.st_atime);
+	if (flags[FLAG_MTIME])
+		sum_add_time(&meta, st.st_mtime);
+	if (flags[FLAG_CTIME])
+		sum_add_time(&meta, st.st_ctime);
+	if (flags[FLAG_XATTRS] &&
+		(S_ISDIR(st.st_mode) || S_ISREG(st.st_mode))) {
+		fd = openat(dirfd, name, 0);
+		if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
+			sum_add_u64(&meta, errno);
+		} else if (fd == -1) {
+			fprintf(stderr, "open failed for %s/%s: %s\n",
+				path_prefix, path, strerror(errno));
+			exit(-1);
+		} else {
+			ret = sum_xattrs(fd, &meta);
+			close(fd);
+			if (ret < 0) {
+				fprintf(stderr,
+					"failed to read xattrs from "
+					"%s/%s: %s\n",
+					path_prefix, path,
+					strerror(-ret));
 				exit(-1);
-			} else {
-				ret = sum_xattrs(fd, &meta);
-				close(fd);
-				if (ret < 0) {
-					fprintf(stderr,
-						"failed to read xattrs from "
-						"%s/%s: %s\n",
-						path_prefix, path,
-						strerror(-ret));
-					exit(-1);
-				}
 			}
 		}
-		if (S_ISDIR(st.st_mode)) {
-			fd = openat(dirfd, namelist[i], 0);
+	}
+	if (S_ISDIR(st.st_mode)) {
+		fd = openat(dirfd, name, 0);
+		if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
+			sum_add_u64(&meta, errno);
+		} else if (fd == -1) {
+			fprintf(stderr, "open failed for %s/%s: %s\n",
+				path_prefix, path, strerror(errno));
+			exit(-1);
+		} else {
+			sum(fd, level + 1, &cs, path_prefix, path);
+			close(fd);
+		}
+	} else if (S_ISREG(st.st_mode)) {
+		sum_add_u64(&meta, st.st_size);
+		if (flags[FLAG_DATA]) {
+			if (verbose)
+				fprintf(stderr, "file %s\n",
+					name);
+			fd = openat(dirfd, name, 0);
 			if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
 				sum_add_u64(&meta, errno);
 			} else if (fd == -1) {
-				fprintf(stderr, "open failed for %s/%s: %s\n",
-					path_prefix, path, strerror(errno));
+				fprintf(stderr,
+					"open failed for %s/%s: %s\n",
+					path_prefix, path,
+					strerror(errno));
 				exit(-1);
-			} else {
-				sum(fd, level + 1, &cs, path_prefix, path);
-				close(fd);
 			}
-		} else if (S_ISREG(st.st_mode)) {
-			sum_add_u64(&meta, st.st_size);
-			if (flags[FLAG_DATA]) {
-				if (verbose)
-					fprintf(stderr, "file %s\n",
-						namelist[i]);
-				fd = openat(dirfd, namelist[i], 0);
-				if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
-					sum_add_u64(&meta, errno);
-				} else if (fd == -1) {
+			if (fd != -1) {
+				ret = sum_file_data(fd, &cs);
+				if (ret < 0) {
 					fprintf(stderr,
-						"open failed for %s/%s: %s\n",
+						"read failed for "
+						"%s/%s: %s\n",
 						path_prefix, path,
 						strerror(errno));
 					exit(-1);
 				}
-				if (fd != -1) {
-					ret = sum_file_data(fd, &cs);
-					if (ret < 0) {
-						fprintf(stderr,
-							"read failed for "
-							"%s/%s: %s\n",
-							path_prefix, path,
-							strerror(errno));
-						exit(-1);
-					}
-					close(fd);
-				}
-			}
-		} else if (S_ISLNK(st.st_mode)) {
-			ret = readlink(namelist[i], buf, sizeof(buf));
-			if (ret == -1) {
-				perror("readlink");
-				exit(-1);
+				close(fd);
 			}
-			sum_add(&cs, buf, ret);
-		} else if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
-			sum_add_u64(&cs, major(st.st_rdev));
-			sum_add_u64(&cs, minor(st.st_rdev));
 		}
-		sum_fini(&cs);
-		sum_fini(&meta);
-		if (gen_manifest || in_manifest) {
-			char *fn;
-			char *m;
-			char *c;
-
-			if (S_ISDIR(st.st_mode))
-				strcat(path, "/");
-			fn = escape(path);
-			m = sum_to_string(&meta);
-			c = sum_to_string(&cs);
-
-			if (gen_manifest)
-				fprintf(out_fp, "%s %s %s\n", fn, m, c);
-			if (in_manifest)
-				check_manifest(fn, m, c, 0);
-			free(c);
-			free(m);
-			free(fn);
+	} else if (S_ISLNK(st.st_mode)) {
+		ret = readlink(name, buf, sizeof(buf));
+		if (ret == -1) {
+			perror("readlink");
+			exit(-1);
 		}
-		sum_add_sum(dircs, &cs);
-		sum_add_sum(dircs, &meta);
-next:
-		free(path);
+		sum_add(&cs, buf, ret);
+	} else if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
+		sum_add_u64(&cs, major(st.st_rdev));
+		sum_add_u64(&cs, minor(st.st_rdev));
 	}
+	sum_fini(&cs);
+	sum_fini(&meta);
+	if (gen_manifest || in_manifest) {
+		char *fn;
+		char *m;
+		char *c;
+
+		if (S_ISDIR(st.st_mode))
+			strcat(path, "/");
+		fn = escape(path);
+		m = sum_to_string(&meta);
+		c = sum_to_string(&cs);
+
+		if (gen_manifest)
+			fprintf(out_fp, "%s %s %s\n", fn, m, c);
+		if (in_manifest)
+			check_manifest(fn, m, c, 0);
+		free(c);
+		free(m);
+		free(fn);
+	}
+	sum_add_sum(dircs, &cs);
+	sum_add_sum(dircs, &meta);
+
+out:
+	free(path);
 }
 
 int
-- 
2.20.1

[PATCH 3/6] src/fssum: Add flag -R for non-recursive mode

[Arvind Raghavan]

By default, fssum walks the input directory recursively. This patch adds
a non-recursive mode which is useful because POSIX standards dictate
that fsyncing a directory only guarantees its immediate dirents are
synced.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
---
 src/fssum.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 81e5cf5d..018ad4a6 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -56,6 +56,7 @@ typedef int (*sum_file_data_t)(int fd, sum_t *dst);
 
 int gen_manifest = 0;
 int in_manifest = 0;
+int recursive = 1;
 char *checksum = NULL;
 struct excludes *excludes;
 int n_excludes = 0;
@@ -151,6 +152,7 @@ usage(void)
 	fprintf(stderr, "    -n           : reset all flags\n");
 	fprintf(stderr, "    -N           : set all flags\n");
 	fprintf(stderr, "    -x path      : exclude path when building checksum (multiple ok)\n");
+	fprintf(stderr, "    -R           : traverse dirs non-recursively (recursive is default)\n");
 	fprintf(stderr, "    -h           : this help\n\n");
 	fprintf(stderr, "The default field mask is ugoamCdtES. If the checksum/manifest is read from a\n");
 	fprintf(stderr, "file, the mask is taken from there and the values given on the command line\n");
@@ -634,7 +636,7 @@ sum_one(int dirfd, int level, sum_t *dircs, char *path_prefix,
 			}
 		}
 	}
-	if (S_ISDIR(st.st_mode)) {
+	if (S_ISDIR(st.st_mode) && recursive) {
 		fd = openat(dirfd, name, 0);
 		if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
 			sum_add_u64(&meta, errno);
@@ -728,7 +730,7 @@ main(int argc, char *argv[])
 	int plen;
 	int elen;
 	int n_flags = 0;
-	const char *allopts = "heEfuUgGoOaAmMcCdDtTsSnNw:r:vx:";
+	const char *allopts = "heEfuUgGoOaAmMcCdDtTsSnNRw:r:vx:";
 
 	out_fp = stdout;
 	while ((c = getopt(argc, argv, allopts)) != EOF) {
@@ -736,6 +738,9 @@ main(int argc, char *argv[])
 		case 'f':
 			gen_manifest = 1;
 			break;
+		case 'R':
+			recursive = 0;
+			break;
 		case 'u':
 		case 'U':
 		case 'g':
-- 
2.20.1

[PATCH 4/6] src/fssum: Add a flag for including file size in checksum

[Arvind Raghavan]

File size is currently included by default. It is useful to ignore file
sizes when testing the fsync of a directory, as the data in children is
not guaranteed to be persisted.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
---
 src/fssum.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 018ad4a6..ece0f556 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -76,13 +76,14 @@ enum _flags {
 	FLAG_XATTRS,
 	FLAG_OPEN_ERROR,
 	FLAG_STRUCTURE,
+	FLAG_SIZE,
 	NUM_FLAGS
 };
 
-const char flchar[] = "ugoamcdtes";
+const char flchar[] = "ugoamcdtesz";
 char line[65536];
 
-int flags[NUM_FLAGS] = {1, 1, 1, 1, 1, 0, 1, 1, 0, 0};
+int flags[NUM_FLAGS] = {1, 1, 1, 1, 1, 0, 1, 1, 0, 0, 1};
 
 char *
 getln(char *buf, int size, FILE *fp)
@@ -137,7 +138,7 @@ usage(void)
 	fprintf(stderr, "    -w <file>    : send output to file\n");
 	fprintf(stderr, "    -v           : verbose mode (debugging only)\n");
 	fprintf(stderr, "    -r <file>    : read checksum or manifest from file\n");
-	fprintf(stderr, "    -[ugoamcdtes]: specify which fields to include in checksum calculation.\n");
+	fprintf(stderr, "    -[ugoamcdtesz]: specify which fields to include in checksum calculation.\n");
 	fprintf(stderr, "         u       : include uid\n");
 	fprintf(stderr, "         g       : include gid\n");
 	fprintf(stderr, "         o       : include mode\n");
@@ -148,13 +149,14 @@ usage(void)
 	fprintf(stderr, "         t       : include xattrs\n");
 	fprintf(stderr, "         e       : include open errors (aborts otherwise)\n");
 	fprintf(stderr, "         s       : include block structure (holes)\n");
-	fprintf(stderr, "    -[UGOAMCDTES]: exclude respective field from calculation\n");
+	fprintf(stderr, "         z       : include file size\n");
+	fprintf(stderr, "    -[UGOAMCDTESZ]: exclude respective field from calculation\n");
 	fprintf(stderr, "    -n           : reset all flags\n");
 	fprintf(stderr, "    -N           : set all flags\n");
 	fprintf(stderr, "    -x path      : exclude path when building checksum (multiple ok)\n");
 	fprintf(stderr, "    -R           : traverse dirs non-recursively (recursive is default)\n");
 	fprintf(stderr, "    -h           : this help\n\n");
-	fprintf(stderr, "The default field mask is ugoamCdtES. If the checksum/manifest is read from a\n");
+	fprintf(stderr, "The default field mask is ugoamCdtESz. If the checksum/manifest is read from a\n");
 	fprintf(stderr, "file, the mask is taken from there and the values given on the command line\n");
 	fprintf(stderr, "are ignored.\n");
 	exit(-1);
@@ -649,7 +651,8 @@ sum_one(int dirfd, int level, sum_t *dircs, char *path_prefix,
 			close(fd);
 		}
 	} else if (S_ISREG(st.st_mode)) {
-		sum_add_u64(&meta, st.st_size);
+		if (flags[FLAG_SIZE])
+			sum_add_u64(&meta, st.st_size);
 		if (flags[FLAG_DATA]) {
 			if (verbose)
 				fprintf(stderr, "file %s\n",
@@ -730,7 +733,7 @@ main(int argc, char *argv[])
 	int plen;
 	int elen;
 	int n_flags = 0;
-	const char *allopts = "heEfuUgGoOaAmMcCdDtTsSnNRw:r:vx:";
+	const char *allopts = "heEfuUgGoOaAmMcCdDtTsSzZnNRw:r:vx:";
 
 	out_fp = stdout;
 	while ((c = getopt(argc, argv, allopts)) != EOF) {
@@ -761,6 +764,8 @@ main(int argc, char *argv[])
 		case 'E':
 		case 's':
 		case 'S':
+		case 'z':
+		case 'Z':
 			++n_flags;
 			parse_flag(c);
 			break;
-- 
2.20.1

[PATCH 5/6] src/fssum: Allow single file input

[Arvind Raghavan]

Allow regular links and symlinks to be passed as input to fssum.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
---
 src/fssum.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/fssum.c b/src/fssum.c
index ece0f556..2d1624ca 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -29,6 +29,7 @@
 #include <inttypes.h>
 #include <assert.h>
 #include <endian.h>
+#include <libgen.h>
 
 #define CS_SIZE 16
 #define CHUNKS	128
@@ -884,8 +885,40 @@ main(int argc, char *argv[])
 	if (gen_manifest)
 		fprintf(out_fp, "Flags: %s\n", flagstring);
 
+	struct stat64 path_st;
+	if (fstat64(fd, &path_st)) {
+		perror("fstat");
+		exit(-1);
+	}
+
 	sum_init(&cs);
-	sum(fd, 1, &cs, path, "");
+
+	if (S_ISDIR(path_st.st_mode)) {
+		sum(fd, 1, &cs, path, "");
+	} else if (S_ISREG(path_st.st_mode) || S_ISLNK(path_st.st_mode)) {
+		// Copy because dirname may modify path
+		char* path_copy = alloc(strlen(path));
+		strcpy(path_copy, path);
+
+		char* dir_path = dirname(path);
+		char* name = basename(path_copy);
+
+		int dirfd = open(dir_path, O_RDONLY);
+		if (fd == -1) {
+			fprintf(stderr, "failed to open %s: %s\n", dir_path,
+				strerror(errno));
+			exit(-1);
+		}
+
+		sum_one(dirfd, 1, &cs, dir_path, "", name);
+
+		free(path_copy);
+		close(dirfd);
+	} else {
+		fprintf(stderr, "path must be file or dir: %s", path);
+		exit(-1);
+	}
+
 	sum_fini(&cs);
 
 	close(fd);
-- 
2.20.1

Re: [PATCH 5/6] src/fssum: Allow single file input

[Amir Goldstein]

On Thu, May 21, 2020 at 3:10 AM Arvind Raghavan
<raghavan.arvind@gmail.com> wrote:
>
> Allow regular links and symlinks to be passed as input to fssum.
>
> Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
> Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
> Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
> ---
>  src/fssum.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/src/fssum.c b/src/fssum.c
> index ece0f556..2d1624ca 100644
> --- a/src/fssum.c
> +++ b/src/fssum.c
> @@ -29,6 +29,7 @@
>  #include <inttypes.h>
>  #include <assert.h>
>  #include <endian.h>
> +#include <libgen.h>
>
>  #define CS_SIZE 16
>  #define CHUNKS 128
> @@ -884,8 +885,40 @@ main(int argc, char *argv[])
>         if (gen_manifest)
>                 fprintf(out_fp, "Flags: %s\n", flagstring);
>
> +       struct stat64 path_st;
> +       if (fstat64(fd, &path_st)) {
> +               perror("fstat");
> +               exit(-1);
> +       }
> +
>         sum_init(&cs);
> -       sum(fd, 1, &cs, path, "");
> +
> +       if (S_ISDIR(path_st.st_mode)) {
> +               sum(fd, 1, &cs, path, "");
> +       } else if (S_ISREG(path_st.st_mode) || S_ISLNK(path_st.st_mode)) {
> +               // Copy because dirname may modify path
> +               char* path_copy = alloc(strlen(path));
> +               strcpy(path_copy, path);
> +
> +               char* dir_path = dirname(path);
> +               char* name = basename(path_copy);
> +
> +               int dirfd = open(dir_path, O_RDONLY);
> +               if (fd == -1) {
> +                       fprintf(stderr, "failed to open %s: %s\n", dir_path,
> +                               strerror(errno));
> +                       exit(-1);
> +               }
> +
> +               sum_one(dirfd, 1, &cs, dir_path, "", name);

Instead of all of the above, how about just:
               sum_one(fd, 1, &cs, path, "", "");

From looking at sum_one() code, it seems to me like that will work,
but I may be missing something.
It's not that you *want* the name in the checksum, it is not even
part of the metadata that is being synced with fsync.

Other than that patch set looks excellent.
Very pleasant for review :-)

One little thing is missing from the cover letter -
Which tests did you run to verify these changes do not regress existing
tests?

Feel free to add to all the other patches in the series:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

If you are able to change sum_one() of empty name as I suggested,
feel free to add that to this patch as well.

Thanks,
Amir.

Re: [PATCH 5/6] src/fssum: Allow single file input

[Arvind Raghavan]

On 05/21, Amir Goldstein wrote:
> On Thu, May 21, 2020 at 3:10 AM Arvind Raghavan
> <raghavan.arvind@gmail.com> wrote:
> >
> > Allow regular links and symlinks to be passed as input to fssum.
> >
> > Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
> > Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
> > Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
> > ---
> >  src/fssum.c | 35 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/fssum.c b/src/fssum.c
> > index ece0f556..2d1624ca 100644
> > --- a/src/fssum.c
> > +++ b/src/fssum.c
> > @@ -29,6 +29,7 @@
> >  #include <inttypes.h>
> >  #include <assert.h>
> >  #include <endian.h>
> > +#include <libgen.h>
> >
> >  #define CS_SIZE 16
> >  #define CHUNKS 128
> > @@ -884,8 +885,40 @@ main(int argc, char *argv[])
> >         if (gen_manifest)
> >                 fprintf(out_fp, "Flags: %s\n", flagstring);
> >
> > +       struct stat64 path_st;
> > +       if (fstat64(fd, &path_st)) {
> > +               perror("fstat");
> > +               exit(-1);
> > +       }
> > +
> >         sum_init(&cs);
> > -       sum(fd, 1, &cs, path, "");
> > +
> > +       if (S_ISDIR(path_st.st_mode)) {
> > +               sum(fd, 1, &cs, path, "");
> > +       } else if (S_ISREG(path_st.st_mode) || S_ISLNK(path_st.st_mode)) {
> > +               // Copy because dirname may modify path
> > +               char* path_copy = alloc(strlen(path));
> > +               strcpy(path_copy, path);
> > +
> > +               char* dir_path = dirname(path);
> > +               char* name = basename(path_copy);
> > +
> > +               int dirfd = open(dir_path, O_RDONLY);
> > +               if (fd == -1) {
> > +                       fprintf(stderr, "failed to open %s: %s\n", dir_path,
> > +                               strerror(errno));
> > +                       exit(-1);
> > +               }
> > +
> > +               sum_one(dirfd, 1, &cs, dir_path, "", name);
> 
> Instead of all of the above, how about just:
>                sum_one(fd, 1, &cs, path, "", "");
> 
> From looking at sum_one() code, it seems to me like that will work,
> but I may be missing something.
> It's not that you *want* the name in the checksum, it is not even
> part of the metadata that is being synced with fsync.

The issue here is that we preserved the code from sum which does
all its opens using openat with the parent directory fd and a
filename. Since we're trying to reuse that code I believe we need
to have this somewhat ugly boilerplate.

> Other than that patch set looks excellent.
> Very pleasant for review :-)

Thanks! :)

> One little thing is missing from the cover letter -
> Which tests did you run to verify these changes do not regress existing
> tests?

I just ran the relevant tests and encountered a small issue with
the refactoring patch. This is my bad, since we changed lstat to
use fstatat, we are no longer doing a fchdir which a readlink
call later on relies on. I can fix it by changing the readlink to
a readlinkat.

I'll add that change and add the set of relevant patches to the
cover letter in a V2.

> Feel free to add to all the other patches in the series:
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> If you are able to change sum_one() of empty name as I suggested,
> feel free to add that to this patch as well.
> 
> Thanks,
> Amir.

Re: [PATCH 5/6] src/fssum: Allow single file input

[Amir Goldstein]

On Fri, May 22, 2020 at 4:06 AM Arvind Raghavan
<raghavan.arvind@gmail.com> wrote:
>
> On 05/21, Amir Goldstein wrote:
> > On Thu, May 21, 2020 at 3:10 AM Arvind Raghavan
> > <raghavan.arvind@gmail.com> wrote:
> > >
> > > Allow regular links and symlinks to be passed as input to fssum.
> > >
> > > Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
> > > Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
> > > Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
> > > ---
> > >  src/fssum.c | 35 ++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 34 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/fssum.c b/src/fssum.c
> > > index ece0f556..2d1624ca 100644
> > > --- a/src/fssum.c
> > > +++ b/src/fssum.c
> > > @@ -29,6 +29,7 @@
> > >  #include <inttypes.h>
> > >  #include <assert.h>
> > >  #include <endian.h>
> > > +#include <libgen.h>
> > >
> > >  #define CS_SIZE 16
> > >  #define CHUNKS 128
> > > @@ -884,8 +885,40 @@ main(int argc, char *argv[])
> > >         if (gen_manifest)
> > >                 fprintf(out_fp, "Flags: %s\n", flagstring);
> > >
> > > +       struct stat64 path_st;
> > > +       if (fstat64(fd, &path_st)) {
> > > +               perror("fstat");
> > > +               exit(-1);
> > > +       }
> > > +
> > >         sum_init(&cs);
> > > -       sum(fd, 1, &cs, path, "");
> > > +
> > > +       if (S_ISDIR(path_st.st_mode)) {
> > > +               sum(fd, 1, &cs, path, "");
> > > +       } else if (S_ISREG(path_st.st_mode) || S_ISLNK(path_st.st_mode)) {
> > > +               // Copy because dirname may modify path
> > > +               char* path_copy = alloc(strlen(path));
> > > +               strcpy(path_copy, path);

If you stay with this code please use strdup().

> > > +
> > > +               char* dir_path = dirname(path);
> > > +               char* name = basename(path_copy);
> > > +
> > > +               int dirfd = open(dir_path, O_RDONLY);
> > > +               if (fd == -1) {
> > > +                       fprintf(stderr, "failed to open %s: %s\n", dir_path,
> > > +                               strerror(errno));
> > > +                       exit(-1);
> > > +               }
> > > +
> > > +               sum_one(dirfd, 1, &cs, dir_path, "", name);
> >
> > Instead of all of the above, how about just:
> >                sum_one(fd, 1, &cs, path, "", "");
> >
> > From looking at sum_one() code, it seems to me like that will work,
> > but I may be missing something.
> > It's not that you *want* the name in the checksum, it is not even
> > part of the metadata that is being synced with fsync.
>
> The issue here is that we preserved the code from sum which does
> all its opens using openat with the parent directory fd and a
> filename. Since we're trying to reuse that code I believe we need
> to have this somewhat ugly boilerplate.

Ok. But if you stay with this please add a comment about why
this is done with a hint for the future how to fix this properly.

Or (up to you) you can fix it by calling this helper instead of openat():

int open_one(int dirfd, const char *name)
{
    if (!name || !*name)
        return dup(dirfd);
    return openat(dirfd, name, 0);
}

fstatat() can take empty name with AT_EMPTY_PATH flag.
readlinkat() should be able to take an empty name, but documentation
is not clear whether fd must be O_PATH - need to verify if it works with
non O_PATH fd.

Again, you don't have to do this to get my reviewed-by its just if you
want to and then of course do it in a prep patch, the same one that
gets rid of fchdir and converts to fstatat() and readlinkat().

>
> > Other than that patch set looks excellent.
> > Very pleasant for review :-)
>
> Thanks! :)
>
> > One little thing is missing from the cover letter -
> > Which tests did you run to verify these changes do not regress existing
> > tests?
>
> I just ran the relevant tests and encountered a small issue with
> the refactoring patch. This is my bad, since we changed lstat to
> use fstatat, we are no longer doing a fchdir which a readlink
> call later on relies on. I can fix it by changing the readlink to
> a readlinkat.
>

I see two valid options. please chose the one you like.

1. Revert removal of fchdir. let refactoring be only refactoring.
2. Remove fchdir and convert to fstatat/readlinkat in separate prep patch
    (with or without the empty name support suggested above)

> I'll add that change and add the set of relevant patches to the
> cover letter in a V2.

For patches that did not change from v1 please add my reviewed-by
so I know I do not need to re-review them.

Please include summary of "changes since v1" in cover letter.

Thanks,
Amir.

Re: [PATCH 5/6] src/fssum: Allow single file input

[Arvind Raghavan]

On 05/22, Amir Goldstein wrote:
> On Fri, May 22, 2020 at 4:06 AM Arvind Raghavan
> <raghavan.arvind@gmail.com> wrote:
> >
> > On 05/21, Amir Goldstein wrote:
> > > On Thu, May 21, 2020 at 3:10 AM Arvind Raghavan
> > > <raghavan.arvind@gmail.com> wrote:
> > > >
> > > > Allow regular links and symlinks to be passed as input to fssum.
> > > >
> > > > Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
> > > > Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
> > > > Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
> > > > ---
> > > >  src/fssum.c | 35 ++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 34 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/fssum.c b/src/fssum.c
> > > > index ece0f556..2d1624ca 100644
> > > > --- a/src/fssum.c
> > > > +++ b/src/fssum.c
> > > > @@ -29,6 +29,7 @@
> > > >  #include <inttypes.h>
> > > >  #include <assert.h>
> > > >  #include <endian.h>
> > > > +#include <libgen.h>
> > > >
> > > >  #define CS_SIZE 16
> > > >  #define CHUNKS 128
> > > > @@ -884,8 +885,40 @@ main(int argc, char *argv[])
> > > >         if (gen_manifest)
> > > >                 fprintf(out_fp, "Flags: %s\n", flagstring);
> > > >
> > > > +       struct stat64 path_st;
> > > > +       if (fstat64(fd, &path_st)) {
> > > > +               perror("fstat");
> > > > +               exit(-1);
> > > > +       }
> > > > +
> > > >         sum_init(&cs);
> > > > -       sum(fd, 1, &cs, path, "");
> > > > +
> > > > +       if (S_ISDIR(path_st.st_mode)) {
> > > > +               sum(fd, 1, &cs, path, "");
> > > > +       } else if (S_ISREG(path_st.st_mode) || S_ISLNK(path_st.st_mode)) {
> > > > +               // Copy because dirname may modify path
> > > > +               char* path_copy = alloc(strlen(path));
> > > > +               strcpy(path_copy, path);
> 
> If you stay with this code please use strdup().
> 
> > > > +
> > > > +               char* dir_path = dirname(path);
> > > > +               char* name = basename(path_copy);
> > > > +
> > > > +               int dirfd = open(dir_path, O_RDONLY);
> > > > +               if (fd == -1) {
> > > > +                       fprintf(stderr, "failed to open %s: %s\n", dir_path,
> > > > +                               strerror(errno));
> > > > +                       exit(-1);
> > > > +               }
> > > > +
> > > > +               sum_one(dirfd, 1, &cs, dir_path, "", name);
> > >
> > > Instead of all of the above, how about just:
> > >                sum_one(fd, 1, &cs, path, "", "");
> > >
> > > From looking at sum_one() code, it seems to me like that will work,
> > > but I may be missing something.
> > > It's not that you *want* the name in the checksum, it is not even
> > > part of the metadata that is being synced with fsync.
> >
> > The issue here is that we preserved the code from sum which does
> > all its opens using openat with the parent directory fd and a
> > filename. Since we're trying to reuse that code I believe we need
> > to have this somewhat ugly boilerplate.
> 
> Ok. But if you stay with this please add a comment about why
> this is done with a hint for the future how to fix this properly.
> 
> Or (up to you) you can fix it by calling this helper instead of openat():
> 
> int open_one(int dirfd, const char *name)
> {
>     if (!name || !*name)
>         return dup(dirfd);
>     return openat(dirfd, name, 0);
> }
> 
> fstatat() can take empty name with AT_EMPTY_PATH flag.
> readlinkat() should be able to take an empty name, but documentation
> is not clear whether fd must be O_PATH - need to verify if it works with
> non O_PATH fd.

I might be doing something wrong but I can't seem to get
AT_EMPTY_PATH to work with fstatat. I don't see it on the man
page and when I pass it to fstatat it errors out with invalid
argument.

In the case that fstatat doesn't support AT_EMPTY_PATH, do you
see another way to fix the above code?

Thanks,
Arvind

Re: [PATCH 5/6] src/fssum: Allow single file input

[Amir Goldstein]

On Sun, May 31, 2020 at 9:29 PM Arvind Raghavan
<raghavan.arvind@gmail.com> wrote:
>
> On 05/22, Amir Goldstein wrote:
> > On Fri, May 22, 2020 at 4:06 AM Arvind Raghavan
> > <raghavan.arvind@gmail.com> wrote:
> > >
> > > On 05/21, Amir Goldstein wrote:
> > > > On Thu, May 21, 2020 at 3:10 AM Arvind Raghavan
> > > > <raghavan.arvind@gmail.com> wrote:
> > > > >
> > > > > Allow regular links and symlinks to be passed as input to fssum.
> > > > >
> > > > > Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
> > > > > Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
> > > > > Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
> > > > > ---
> > > > >  src/fssum.c | 35 ++++++++++++++++++++++++++++++++++-
> > > > >  1 file changed, 34 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/src/fssum.c b/src/fssum.c
> > > > > index ece0f556..2d1624ca 100644
> > > > > --- a/src/fssum.c
> > > > > +++ b/src/fssum.c
> > > > > @@ -29,6 +29,7 @@
> > > > >  #include <inttypes.h>
> > > > >  #include <assert.h>
> > > > >  #include <endian.h>
> > > > > +#include <libgen.h>
> > > > >
> > > > >  #define CS_SIZE 16
> > > > >  #define CHUNKS 128
> > > > > @@ -884,8 +885,40 @@ main(int argc, char *argv[])
> > > > >         if (gen_manifest)
> > > > >                 fprintf(out_fp, "Flags: %s\n", flagstring);
> > > > >
> > > > > +       struct stat64 path_st;
> > > > > +       if (fstat64(fd, &path_st)) {
> > > > > +               perror("fstat");
> > > > > +               exit(-1);
> > > > > +       }
> > > > > +
> > > > >         sum_init(&cs);
> > > > > -       sum(fd, 1, &cs, path, "");
> > > > > +
> > > > > +       if (S_ISDIR(path_st.st_mode)) {
> > > > > +               sum(fd, 1, &cs, path, "");
> > > > > +       } else if (S_ISREG(path_st.st_mode) || S_ISLNK(path_st.st_mode)) {
> > > > > +               // Copy because dirname may modify path
> > > > > +               char* path_copy = alloc(strlen(path));
> > > > > +               strcpy(path_copy, path);
> >
> > If you stay with this code please use strdup().
> >
> > > > > +
> > > > > +               char* dir_path = dirname(path);
> > > > > +               char* name = basename(path_copy);
> > > > > +
> > > > > +               int dirfd = open(dir_path, O_RDONLY);
> > > > > +               if (fd == -1) {
> > > > > +                       fprintf(stderr, "failed to open %s: %s\n", dir_path,
> > > > > +                               strerror(errno));
> > > > > +                       exit(-1);
> > > > > +               }
> > > > > +
> > > > > +               sum_one(dirfd, 1, &cs, dir_path, "", name);
> > > >
> > > > Instead of all of the above, how about just:
> > > >                sum_one(fd, 1, &cs, path, "", "");
> > > >
> > > > From looking at sum_one() code, it seems to me like that will work,
> > > > but I may be missing something.
> > > > It's not that you *want* the name in the checksum, it is not even
> > > > part of the metadata that is being synced with fsync.
> > >
> > > The issue here is that we preserved the code from sum which does
> > > all its opens using openat with the parent directory fd and a
> > > filename. Since we're trying to reuse that code I believe we need
> > > to have this somewhat ugly boilerplate.
> >
> > Ok. But if you stay with this please add a comment about why
> > this is done with a hint for the future how to fix this properly.
> >
> > Or (up to you) you can fix it by calling this helper instead of openat():
> >
> > int open_one(int dirfd, const char *name)
> > {
> >     if (!name || !*name)
> >         return dup(dirfd);
> >     return openat(dirfd, name, 0);
> > }
> >
> > fstatat() can take empty name with AT_EMPTY_PATH flag.
> > readlinkat() should be able to take an empty name, but documentation
> > is not clear whether fd must be O_PATH - need to verify if it works with
> > non O_PATH fd.
>
> I might be doing something wrong but I can't seem to get
> AT_EMPTY_PATH to work with fstatat. I don't see it on the man
> page and when I pass it to fstatat it errors out with invalid
> argument.
>

https://www.man7.org/linux/man-pages/man2/stat.2.html

Says:
       AT_EMPTY_PATH (since Linux 2.6.39)
... This flag is Linux-specific; define _GNU_SOURCE to obtain its definition.

And I see that fssum.c does define _GNU_SOURCE, so not sure what the
problem is.

> In the case that fstatat doesn't support AT_EMPTY_PATH, do you
> see another way to fix the above code?
>

Yes, you can pass path_st to sum_one() you already have it.

But I don't think that you or anyone that cares about xfstests is running
a kernel older than 2.6.39...

Thanks,
Amir.

Re: [PATCH 5/6] src/fssum: Allow single file input

[Arvind Raghavan]

On 05/31, Amir Goldstein wrote:
> > I might be doing something wrong but I can't seem to get
> > AT_EMPTY_PATH to work with fstatat. I don't see it on the man
> > page and when I pass it to fstatat it errors out with invalid
> > argument.
> >
> 
> https://www.man7.org/linux/man-pages/man2/stat.2.html
> 
> Says:
>        AT_EMPTY_PATH (since Linux 2.6.39)
> ... This flag is Linux-specific; define _GNU_SOURCE to obtain its definition.
> 
> And I see that fssum.c does define _GNU_SOURCE, so not sure what the
> problem is.

Ah yes, it is in the documentation, and I've got it working now!
Must've been some other issue I had... I apologize for the delay,
I'll send out a v2 shortly.

Thanks,
Arvind

[PATCH 6/6] src/fssum: Fix whitespace in usage

[Arvind Raghavan]

Aligns the usage whitespace properly.

Signed-off-by: Arvind Raghavan <raghavan.arvind@gmail.com>
Signed-off-by: Jayashree Mohan <jaya@cs.utexas.edu>
Signed-off-by: Vijay Chidambaram <vijay@cs.utexas.edu>
---
 src/fssum.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/fssum.c b/src/fssum.c
index 2d1624ca..eac2a492 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -135,28 +135,28 @@ usage(void)
 {
 	fprintf(stderr, "usage: fssum <options> <path>\n");
 	fprintf(stderr, "  options:\n");
-	fprintf(stderr, "    -f           : write out a full manifest file\n");
-	fprintf(stderr, "    -w <file>    : send output to file\n");
-	fprintf(stderr, "    -v           : verbose mode (debugging only)\n");
-	fprintf(stderr, "    -r <file>    : read checksum or manifest from file\n");
+	fprintf(stderr, "    -f            : write out a full manifest file\n");
+	fprintf(stderr, "    -w <file>     : send output to file\n");
+	fprintf(stderr, "    -v            : verbose mode (debugging only)\n");
+	fprintf(stderr, "    -r <file>     : read checksum or manifest from file\n");
 	fprintf(stderr, "    -[ugoamcdtesz]: specify which fields to include in checksum calculation.\n");
-	fprintf(stderr, "         u       : include uid\n");
-	fprintf(stderr, "         g       : include gid\n");
-	fprintf(stderr, "         o       : include mode\n");
-	fprintf(stderr, "         m       : include mtime\n");
-	fprintf(stderr, "         a       : include atime\n");
-	fprintf(stderr, "         c       : include ctime\n");
-	fprintf(stderr, "         d       : include file data\n");
-	fprintf(stderr, "         t       : include xattrs\n");
-	fprintf(stderr, "         e       : include open errors (aborts otherwise)\n");
-	fprintf(stderr, "         s       : include block structure (holes)\n");
-	fprintf(stderr, "         z       : include file size\n");
+	fprintf(stderr, "         u        : include uid\n");
+	fprintf(stderr, "         g        : include gid\n");
+	fprintf(stderr, "         o        : include mode\n");
+	fprintf(stderr, "         m        : include mtime\n");
+	fprintf(stderr, "         a        : include atime\n");
+	fprintf(stderr, "         c        : include ctime\n");
+	fprintf(stderr, "         d        : include file data\n");
+	fprintf(stderr, "         t        : include xattrs\n");
+	fprintf(stderr, "         e        : include open errors (aborts otherwise)\n");
+	fprintf(stderr, "         s        : include block structure (holes)\n");
+	fprintf(stderr, "         z        : include file size\n");
 	fprintf(stderr, "    -[UGOAMCDTESZ]: exclude respective field from calculation\n");
-	fprintf(stderr, "    -n           : reset all flags\n");
-	fprintf(stderr, "    -N           : set all flags\n");
-	fprintf(stderr, "    -x path      : exclude path when building checksum (multiple ok)\n");
-	fprintf(stderr, "    -R           : traverse dirs non-recursively (recursive is default)\n");
-	fprintf(stderr, "    -h           : this help\n\n");
+	fprintf(stderr, "    -n            : reset all flags\n");
+	fprintf(stderr, "    -N            : set all flags\n");
+	fprintf(stderr, "    -x path       : exclude path when building checksum (multiple ok)\n");
+	fprintf(stderr, "    -R            : traverse dirs non-recursively (recursive is default)\n");
+	fprintf(stderr, "    -h            : this help\n\n");
 	fprintf(stderr, "The default field mask is ugoamCdtESz. If the checksum/manifest is read from a\n");
 	fprintf(stderr, "file, the mask is taken from there and the values given on the command line\n");
 	fprintf(stderr, "are ignored.\n");
-- 
2.20.1