Cleanup/fix: gensquashfs: split directory scanning from xattr scanning

On the one hand, this commit cleanes the code a bit by splitting
the "scan directory contents" code from the "scan xattrs from
directory contents" and moving the later in a seperate file.

On the other hand, the xattr scanning is now done *after* the fstree
is post processed, which includes sorting it. This way, the xattrs
are always added in a deterministic, reproducible order.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>

4 files changed, 223 insertions, 213 deletions

diff --git a/bin/gensquashfs/dirscan.c b/bin/gensquashfs/dirscan.c
index dbc862c..bb6d2c3 100644
--- a/bin/gensquashfs/dirscan.c
+++ b/bin/gensquashfs/dirscan.c
@@ -1,201 +1,19 @@
 /* SPDX-License-Identifier: GPL-3.0-or-later */
 /*
- * fstree_from_dir.c
+ * dirscan.c
  *
  * Copyright (C) 2019 David Oberhollenzer <goliath@infraroot.at>
  */
 #include "mkfs.h"
 
-#ifdef HAVE_SYS_XATTR_H
-static char *get_full_path(const char *prefix, tree_node_t *node)
-{
-	char *path = NULL, *new = NULL;
-	size_t path_len, prefix_len;
-	int ret;
-
-	path = fstree_get_path(node);
-	if (path == NULL)
-		goto fail;
-
-	ret = canonicalize_name(path);
-	assert(ret == 0);
-
-	path_len = strlen(path);
-	prefix_len = strlen(prefix);
-
-	while (prefix_len > 0 && prefix[prefix_len - 1] == '/')
-		--prefix_len;
-
-	if (prefix_len > 0) {
-		new = realloc(path, path_len + prefix_len + 2);
-		if (new == NULL)
-			goto fail;
-
-		path = new;
-
-		memmove(path + prefix_len + 1, path, path_len + 1);
-		memcpy(path, prefix, prefix_len);
-		path[prefix_len] = '/';
-	}
-
-	return path;
-fail:
-	perror("getting full path for xattr scan");
-	free(path);
-	return NULL;
-}
-
-static int xattr_from_path(sqfs_xattr_writer_t *xwr, const char *path)
-{
-	char *key, *value = NULL, *buffer = NULL;
-	ssize_t buflen, vallen, keylen;
-	int ret;
-
-	buflen = llistxattr(path, NULL, 0);
-	if (buflen < 0) {
-		fprintf(stderr, "llistxattr %s: %s", path, strerror(errno));
-		return -1;
-	}
-
-	if (buflen == 0)
-		return 0;
-
-	buffer = malloc(buflen);
-	if (buffer == NULL) {
-		perror("xattr name buffer");
-		return -1;
-	}
-
-	buflen = llistxattr(path, buffer, buflen);
-	if (buflen == -1) {
-		fprintf(stderr, "llistxattr %s: %s", path, strerror(errno));
-		goto fail;
-	}
-
-	key = buffer;
-	while (buflen > 0) {
-		vallen = lgetxattr(path, key, NULL, 0);
-		if (vallen == -1) {
-			fprintf(stderr, "lgetxattr %s: %s",
-				path, strerror(errno));
-			goto fail;
-		}
-
-		if (vallen > 0) {
-			value = calloc(1, vallen);
-			if (value == NULL) {
-				perror("allocating xattr value buffer");
-				goto fail;
-			}
-
-			vallen = lgetxattr(path, key, value, vallen);
-			if (vallen == -1) {
-				fprintf(stderr, "lgetxattr %s: %s\n",
-					path, strerror(errno));
-				goto fail;
-			}
-
-			ret = sqfs_xattr_writer_add(xwr, key, value, vallen);
-			if (ret) {
-				sqfs_perror(path,
-					    "storing xattr key-value pairs",
-					    ret);
-				goto fail;
-			}
-
-			free(value);
-			value = NULL;
-		}
-
-		keylen = strlen(key) + 1;
-		buflen -= keylen;
-		key += keylen;
-	}
-
-	free(buffer);
-	return 0;
-fail:
-	free(value);
-	free(buffer);
-	return -1;
-}
-#endif
-
 #ifdef _WIN32
-int fstree_from_dir(fstree_t *fs, const char *path, void *selinux_handle,
-		    sqfs_xattr_writer_t *xwr, unsigned int flags)
+int fstree_from_dir(fstree_t *fs, const char *path, unsigned int flags)
 {
-	(void)fs; (void)path; (void)selinux_handle; (void)xwr; (void)flags;
+	(void)fs; (void)path; (void)flags;
 	fputs("Packing a directory is not supported on Windows.\n", stderr);
 	return -1;
 }
 #else
-static int xattr_xcan_dfs(const char *path_prefix, void *selinux_handle,
-			  sqfs_xattr_writer_t *xwr, unsigned int flags,
-			  tree_node_t *node)
-{
-	char *path;
-	int ret;
-
-	ret = sqfs_xattr_writer_begin(xwr);
-	if (ret) {
-		sqfs_perror(node->name, "recoding xattr key-value pairs\n",
-			    ret);
-		return -1;
-	}
-
-#ifdef HAVE_SYS_XATTR_H
-	if (flags & DIR_SCAN_READ_XATTR) {
-		path = get_full_path(path_prefix, node);
-		if (path == NULL)
-			return -1;
-
-		ret = xattr_from_path(xwr, path);
-		free(path);
-
-		if (ret)
-			return -1;
-	}
-#else
-	(void)path_prefix;
-#endif
-
-	if (selinux_handle != NULL) {
-		path = fstree_get_path(node);
-		if (path == NULL) {
-			perror("reconstructing absolute path");
-			return -1;
-		}
-
-		ret = selinux_relable_node(selinux_handle, xwr, node, path);
-		free(path);
-
-		if (ret)
-			return -1;
-	}
-
-	if (sqfs_xattr_writer_end(xwr, &node->xattr_idx)) {
-		sqfs_perror(node->name, "completing xattr key-value pairs",
-			    ret);
-		return -1;
-	}
-
-	if (S_ISDIR(node->mode)) {
-		node = node->data.dir.children;
-
-		while (node != NULL) {
-			if (xattr_xcan_dfs(path_prefix, selinux_handle, xwr,
-					   flags, node)) {
-				return -1;
-			}
-
-			node = node->next;
-		}
-	}
-
-	return 0;
-}
-
 static int populate_dir(int dir_fd, fstree_t *fs, tree_node_t *root,
 			dev_t devstart, unsigned int flags)
 {
@@ -289,8 +107,7 @@ fail:
 	return -1;
 }
 
-int fstree_from_dir(fstree_t *fs, const char *path, void *selinux_handle,
-		    sqfs_xattr_writer_t *xwr, unsigned int flags)
+int fstree_from_dir(fstree_t *fs, const char *path, unsigned int flags)
 {
 	struct stat sb;
 	int fd;
@@ -307,15 +124,6 @@ int fstree_from_dir(fstree_t *fs, const char *path, void *selinux_handle,
 		return -1;
 	}
 
-	if (populate_dir(fd, fs, fs->root, sb.st_dev, flags))
-		return -1;
-
-	if (xwr != NULL && (selinux_handle != NULL ||
-			    (flags & DIR_SCAN_READ_XATTR))) {
-		if (xattr_xcan_dfs(path, selinux_handle, xwr, flags, fs->root))
-			return -1;
-	}
-
-	return 0;
+	return populate_dir(fd, fs, fs->root, sb.st_dev, flags);
 }
 #endif
diff --git a/bin/gensquashfs/dirscan_xattr.c b/bin/gensquashfs/dirscan_xattr.c
new file mode 100644
index 0000000..3995a19
--- /dev/null
+++ b/bin/gensquashfs/dirscan_xattr.c
@@ -0,0 +1,200 @@
+/* SPDX-License-Identifier: GPL-3.0-or-later */
+/*
+ * dirscan_xattr.c
+ *
+ * Copyright (C) 2019 David Oberhollenzer <goliath@infraroot.at>
+ */
+#include "mkfs.h"
+
+#ifdef HAVE_SYS_XATTR_H
+static char *get_full_path(const char *prefix, tree_node_t *node)
+{
+	char *path = NULL, *new = NULL;
+	size_t path_len, prefix_len;
+	int ret;
+
+	path = fstree_get_path(node);
+	if (path == NULL)
+		goto fail;
+
+	ret = canonicalize_name(path);
+	assert(ret == 0);
+
+	path_len = strlen(path);
+	prefix_len = strlen(prefix);
+
+	while (prefix_len > 0 && prefix[prefix_len - 1] == '/')
+		--prefix_len;
+
+	if (prefix_len > 0) {
+		new = realloc(path, path_len + prefix_len + 2);
+		if (new == NULL)
+			goto fail;
+
+		path = new;
+
+		memmove(path + prefix_len + 1, path, path_len + 1);
+		memcpy(path, prefix, prefix_len);
+		path[prefix_len] = '/';
+	}
+
+	return path;
+fail:
+	perror("getting full path for xattr scan");
+	free(path);
+	return NULL;
+}
+
+static int xattr_from_path(sqfs_xattr_writer_t *xwr, const char *path)
+{
+	char *key, *value = NULL, *buffer = NULL;
+	ssize_t buflen, vallen, keylen;
+	int ret;
+
+	buflen = llistxattr(path, NULL, 0);
+	if (buflen < 0) {
+		fprintf(stderr, "llistxattr %s: %s", path, strerror(errno));
+		return -1;
+	}
+
+	if (buflen == 0)
+		return 0;
+
+	buffer = malloc(buflen);
+	if (buffer == NULL) {
+		perror("xattr name buffer");
+		return -1;
+	}
+
+	buflen = llistxattr(path, buffer, buflen);
+	if (buflen == -1) {
+		fprintf(stderr, "llistxattr %s: %s", path, strerror(errno));
+		goto fail;
+	}
+
+	key = buffer;
+	while (buflen > 0) {
+		vallen = lgetxattr(path, key, NULL, 0);
+		if (vallen == -1) {
+			fprintf(stderr, "lgetxattr %s: %s",
+				path, strerror(errno));
+			goto fail;
+		}
+
+		if (vallen > 0) {
+			value = calloc(1, vallen);
+			if (value == NULL) {
+				perror("allocating xattr value buffer");
+				goto fail;
+			}
+
+			vallen = lgetxattr(path, key, value, vallen);
+			if (vallen == -1) {
+				fprintf(stderr, "lgetxattr %s: %s\n",
+					path, strerror(errno));
+				goto fail;
+			}
+
+			ret = sqfs_xattr_writer_add(xwr, key, value, vallen);
+			if (ret) {
+				sqfs_perror(path,
+					    "storing xattr key-value pairs",
+					    ret);
+				goto fail;
+			}
+
+			free(value);
+			value = NULL;
+		}
+
+		keylen = strlen(key) + 1;
+		buflen -= keylen;
+		key += keylen;
+	}
+
+	free(buffer);
+	return 0;
+fail:
+	free(value);
+	free(buffer);
+	return -1;
+}
+#endif
+
+static int xattr_xcan_dfs(const char *path_prefix, void *selinux_handle,
+			  sqfs_xattr_writer_t *xwr, unsigned int flags,
+			  tree_node_t *node)
+{
+	char *path;
+	int ret;
+
+	ret = sqfs_xattr_writer_begin(xwr);
+	if (ret) {
+		sqfs_perror(node->name, "recoding xattr key-value pairs\n",
+			    ret);
+		return -1;
+	}
+
+#ifdef HAVE_SYS_XATTR_H
+	if (flags & DIR_SCAN_READ_XATTR) {
+		path = get_full_path(path_prefix, node);
+		if (path == NULL)
+			return -1;
+
+		ret = xattr_from_path(xwr, path);
+		free(path);
+
+		if (ret)
+			return -1;
+	}
+#else
+	(void)path_prefix;
+#endif
+
+	if (selinux_handle != NULL) {
+		path = fstree_get_path(node);
+		if (path == NULL) {
+			perror("reconstructing absolute path");
+			return -1;
+		}
+
+		ret = selinux_relable_node(selinux_handle, xwr, node, path);
+		free(path);
+
+		if (ret)
+			return -1;
+	}
+
+	if (sqfs_xattr_writer_end(xwr, &node->xattr_idx)) {
+		sqfs_perror(node->name, "completing xattr key-value pairs",
+			    ret);
+		return -1;
+	}
+
+	if (S_ISDIR(node->mode)) {
+		node = node->data.dir.children;
+
+		while (node != NULL) {
+			if (xattr_xcan_dfs(path_prefix, selinux_handle, xwr,
+					   flags, node)) {
+				return -1;
+			}
+
+			node = node->next;
+		}
+	}
+
+	return 0;
+}
+
+int xattrs_from_dir(fstree_t *fs, const char *path, void *selinux_handle,
+		    sqfs_xattr_writer_t *xwr, unsigned int flags)
+{
+	if (xwr == NULL)
+		return 0;
+
+	if (selinux_handle == NULL && !(flags & DIR_SCAN_READ_XATTR))
+		return 0;
+
+	return xattr_xcan_dfs(path, selinux_handle, xwr, flags, fs->root);
+}
diff --git a/bin/gensquashfs/mkfs.c b/bin/gensquashfs/mkfs.c
index b1542d1..87778d9 100644
--- a/bin/gensquashfs/mkfs.c
+++ b/bin/gensquashfs/mkfs.c
@@ -150,11 +150,6 @@ static int read_fstree(fstree_t *fs, options_t *opt, sqfs_xattr_writer_t *xwr,
 	FILE *fp;
 	int ret;
 
-	if (opt->infile == NULL) {
-		return fstree_from_dir(fs, opt->packdir, selinux_handle,
-				       xwr, opt->dirscan_flags);
-	}
-
 	fp = fopen(opt->infile, "rb");
 	if (fp == NULL) {
 		perror(opt->infile);
@@ -203,23 +198,27 @@ int main(int argc, char **argv)
 			goto out;
 	}
 
-	if (read_fstree(&sqfs.fs, &opt, sqfs.xwr, sehnd)) {
-		if (sehnd != NULL)
-			selinux_close_context_file(sehnd);
-		goto out;
+	if (opt.infile == NULL) {
+		if (fstree_from_dir(&sqfs.fs, opt.packdir, opt.dirscan_flags))
+			goto out;
+	} else {
+		if (read_fstree(&sqfs.fs, &opt, sqfs.xwr, sehnd))
+			goto out;
 	}
 
 	if (opt.force_uid || opt.force_gid)
 		override_owner_dfs(&opt, sqfs.fs.root);
 
-	if (sehnd != NULL) {
-		selinux_close_context_file(sehnd);
-		sehnd = NULL;
-	}
-
 	if (fstree_post_process(&sqfs.fs))
 		goto out;
 
+	if (opt.infile == NULL) {
+		if (xattrs_from_dir(&sqfs.fs, opt.packdir, sehnd,
+				    sqfs.xwr, opt.dirscan_flags)) {
+			goto out;
+		}
+	}
+
 	if (pack_files(sqfs.data, &sqfs.fs, &opt))
 		goto out;
 
@@ -229,5 +228,7 @@ int main(int argc, char **argv)
 	status = EXIT_SUCCESS;
 out:
 	sqfs_writer_cleanup(&sqfs, status);
+	if (sehnd != NULL)
+		selinux_close_context_file(sehnd);
 	return status;
 }
diff --git a/bin/gensquashfs/mkfs.h b/bin/gensquashfs/mkfs.h
index 9a36d8c..6aa0c87 100644
--- a/bin/gensquashfs/mkfs.h
+++ b/bin/gensquashfs/mkfs.h
@@ -63,9 +63,10 @@ enum {
 
 void process_command_line(options_t *opt, int argc, char **argv);
 
-int fstree_from_dir(fstree_t *fs, const char *path, void *selinux_handle,
-		    sqfs_xattr_writer_t *xwr, unsigned int flags);
+int fstree_from_dir(fstree_t *fs, const char *path, unsigned int flags);
 
+int xattrs_from_dir(fstree_t *fs, const char *path, void *selinux_handle,
+		    sqfs_xattr_writer_t *xwr, unsigned int flags);
 
 void *selinux_open_context_file(const char *filename);