On Mon, Jun 22, 2020 at 2:16 AM Arvind Raghavan
<raghavan.arvind@xxxxxxxxx> wrote:
>
> 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.
It is VERY hard to see in review that this does not change functionality.
As opposed to the attached patches.
I know if we were working with modern code review tools, no need for
those cheap tricks, although using a helper name argument in the previous
patch would have helped in any review tool.
Thanks,
Amir.
From 0c8692f28dacbcf5e0dae2b21774706946e54510 Mon Sep 17 00:00:00 2001
From: Arvind Raghavan <raghavan.arvind@xxxxxxxxx>
Date: Sun, 21 Jun 2020 19:15:44 -0400
Subject: [PATCH 1/2] src/fssum: Refactoring changes for recursive traversal
Refactoring changes needed for recursive traversal and single file
input. Makes fstat and readlink use the 'at' alternatives, and creates a
helper function for opening files.
Signed-off-by: Arvind Raghavan <raghavan.arvind@xxxxxxxxx>
Signed-off-by: Jayashree Mohan <jaya@xxxxxxxxxxxxx>
Signed-off-by: Vijay Chidambaram <vijay@xxxxxxxxxxxxx>
---
src/fssum.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/src/fssum.c b/src/fssum.c
index 30f456c2..6192f109 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -503,6 +503,13 @@ malformed:
excess_file(fn);
}
+int open_one(int dirfd, const char *name)
+{
+ if (!name || !*name)
+ return dup(dirfd);
+ return openat(dirfd, name, 0);
+}
+
void
sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
{
@@ -552,23 +559,19 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
sum_t cs;
sum_t meta;
char *path;
+ char *name = namelist[i];
sum_init(&cs);
sum_init(&meta);
- path = alloc(strlen(path_in) + strlen(namelist[i]) + 3);
- sprintf(path, "%s/%s", path_in, namelist[i]);
+ 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 next;
}
- ret = fchdir(dirfd);
- if (ret == -1) {
- perror("fchdir");
- exit(-1);
- }
- ret = lstat64(namelist[i], &st);
+ ret = fstatat64(dirfd, name, &st, AT_SYMLINK_NOFOLLOW);
if (ret) {
fprintf(stderr, "stat failed for %s/%s: %s\n",
path_prefix, path, strerror(errno));
@@ -580,7 +583,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
goto next;
sum_add_u64(&meta, level);
- sum_add(&meta, namelist[i], strlen(namelist[i]));
+ sum_add(&meta, name, strlen(name));
if (!S_ISDIR(st.st_mode))
sum_add_u64(&meta, st.st_nlink);
if (flags[FLAG_UID])
@@ -597,7 +600,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
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);
+ fd = open_one(dirfd, name);
if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
sum_add_u64(&meta, errno);
} else if (fd == -1) {
@@ -618,7 +621,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
}
}
if (S_ISDIR(st.st_mode)) {
- fd = openat(dirfd, namelist[i], 0);
+ fd = open_one(dirfd, name);
if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
sum_add_u64(&meta, errno);
} else if (fd == -1) {
@@ -633,9 +636,8 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
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);
+ fprintf(stderr, "file %s\n", name);
+ fd = open_one(dirfd, name);
if (fd == -1 && flags[FLAG_OPEN_ERROR]) {
sum_add_u64(&meta, errno);
} else if (fd == -1) {
@@ -659,7 +661,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
}
}
} else if (S_ISLNK(st.st_mode)) {
- ret = readlink(namelist[i], buf, sizeof(buf));
+ ret = readlinkat(dirfd, name, buf, sizeof(buf));
if (ret == -1) {
perror("readlink");
exit(-1);
--
2.17.1
From c54d1f8fe91cffe9685268ef5717e999da5a7391 Mon Sep 17 00:00:00 2001
From: Arvind Raghavan <raghavan.arvind@xxxxxxxxx>
Date: Sun, 21 Jun 2020 19:16:03 -0400
Subject: [PATCH 2/2] src/fssum: Recursive traversal refactoring
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@xxxxxxxxx>
Signed-off-by: Jayashree Mohan <jaya@xxxxxxxxxxxxx>
Signed-off-by: Vijay Chidambaram <vijay@xxxxxxxxxxxxx>
---
src/fssum.c | 44 ++++++++++++++++++++++++++++++++++----------
1 file changed, 34 insertions(+), 10 deletions(-)
diff --git a/src/fssum.c b/src/fssum.c
index 6192f109..7bfa8c4f 100644
--- a/src/fssum.c
+++ b/src/fssum.c
@@ -510,6 +510,10 @@ int open_one(int dirfd, const char *name)
return openat(dirfd, name, 0);
}
+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)
{
@@ -520,8 +524,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)) {
@@ -556,10 +558,36 @@ 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;
+
+ ret = fstatat64(dirfd, namelist[i], &st, AT_SYMLINK_NOFOLLOW);
+ if (ret) {
+ 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)
+ 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)
+{
+ /* TEMPORARY NESTING WILL BE REMOVED SOON */
+ {
sum_t cs;
sum_t meta;
- char *path;
- char *name = namelist[i];
+ int fd;
+ int ret;
+ int excl;
+ char* path;
+ struct stat64 st;
sum_init(&cs);
sum_init(&meta);
@@ -568,7 +596,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
for (excl = 0; excl < n_excludes; ++excl) {
if (strncmp(excludes[excl].path, path,
excludes[excl].len) == 0)
- goto next;
+ goto out;
}
ret = fstatat64(dirfd, name, &st, AT_SYMLINK_NOFOLLOW);
@@ -578,10 +606,6 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
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, name, strlen(name));
if (!S_ISDIR(st.st_mode))
@@ -694,7 +718,7 @@ sum(int dirfd, int level, sum_t *dircs, char *path_prefix, char *path_in)
}
sum_add_sum(dircs, &cs);
sum_add_sum(dircs, &meta);
-next:
+out:
free(path);
}
}
--
2.17.1
