diagnose.c: refactor to safely use 'd_type'
Refactor usage of the 'd_type' property of 'struct dirent' in 'diagnose.c' to instead utilize the compatibility macro 'DTYPE()'. On systems where 'd_type' is not present in 'struct dirent', this macro will always return 'DT_UNKNOWN'. In that case, instead fall back on using the 'stat.st_mode' to determine whether the dirent points to a dir, file, or link. Additionally, add a test to 't0092-diagnose.sh' to verify that files (e.g., loose objects) are counted properly. Note that the new function 'get_dtype()' is based on 'resolve_dtype()' in 'dir.c' (which itself was refactored from a prior 'get_dtype()' in ad6f2157f9 (dir: restructure in a way to avoid passing around a struct dirent, 2020-01-16)), but differs in that it is meant for use on arbitrary files, such as those inside the '.git' dir. Because of this, it does not search the index for a matching entry to derive the 'd_type'. Reported-by: Randall S. Becker <rsbecker@nexbridge.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Victoria Dye committed
Sep 17, 2022 at 18:16 UTC
cb98e1d50a7a4a84b76f72dad694d49d2276eef3
2 files changed
+68
-14
diagnose.c
+56
-14
@@ -66,17 +66,53 @@ static int dir_file_stats(struct object_directory *object_dir, void *data)
66
return 0;
67
}
68
69
-static int count_files(char *path)
69
+/*
70
+ * Get the d_type of a dirent. If the d_type is unknown, derive it from
71
+ * stat.st_mode.
72
+ *
73
+ * Note that 'path' is assumed to have a trailing slash. It is also modified
74
+ * in-place during the execution of the function, but is then reverted to its
75
+ * original value before returning.
76
+ */
77
+static unsigned char get_dtype(struct dirent *e, struct strbuf *path)
78
{
71
- DIR *dir = opendir(path);
79
+ struct stat st;
80
+ unsigned char dtype = DTYPE(e);
81
+ size_t base_path_len;
82
+
83
+ if (dtype != DT_UNKNOWN)
84
+ return dtype;
85
+
86
+ /* d_type unknown in dirent, try to fall back on lstat results */
87
+ base_path_len = path->len;
88
+ strbuf_addstr(path, e->d_name);
89
+ if (lstat(path->buf, &st))
90
+ goto cleanup;
91
+
92
+ /* determine d_type from st_mode */
93
+ if (S_ISREG(st.st_mode))
94
+ dtype = DT_REG;
95
+ else if (S_ISDIR(st.st_mode))
96
+ dtype = DT_DIR;
97
+ else if (S_ISLNK(st.st_mode))
98
+ dtype = DT_LNK;
99
+
100
+cleanup:
101
+ strbuf_setlen(path, base_path_len);
102
+ return dtype;
103
+}
104
+
105
+static int count_files(struct strbuf *path)
106
+{
107
+ DIR *dir = opendir(path->buf);
108
struct dirent *e;
109
int count = 0;
110
111
if (!dir)
112
return 0;
113
78
- while ((e = readdir(dir)) != NULL)
79
- if (!is_dot_or_dotdot(e->d_name) && e->d_type == DT_REG)
114
+ while ((e = readdir_skip_dot_and_dotdot(dir)) != NULL)
115
+ if (get_dtype(e, path) == DT_REG)
116
count++;
117
118
closedir(dir);
@@ -104,13 +140,13 @@ static void loose_objs_stats(struct strbuf *buf, const char *path)
140
strbuf_addch(&count_path, '/');
141
base_path_len = count_path.len;
142
107
- while ((e = readdir(dir)) != NULL)
108
- if (!is_dot_or_dotdot(e->d_name) &&
109
- e->d_type == DT_DIR && strlen(e->d_name) == 2 &&
143
+ while ((e = readdir_skip_dot_and_dotdot(dir)) != NULL)
144
+ if (get_dtype(e, &count_path) == DT_DIR &&
145
+ strlen(e->d_name) == 2 &&
146
!hex_to_bytes(&c, e->d_name, 1)) {
147
strbuf_setlen(&count_path, base_path_len);
112
- strbuf_addstr(&count_path, e->d_name);
113
- total += (count = count_files(count_path.buf));
148
+ strbuf_addf(&count_path, "%s/", e->d_name);
149
+ total += (count = count_files(&count_path));
150
strbuf_addf(buf, "%s : %7d files\n", e->d_name, count);
151
}
152
@@ -144,22 +180,28 @@ static int add_directory_to_archiver(struct strvec *archiver_args,
180
len = buf.len;
181
strvec_pushf(archiver_args, "--prefix=%s", buf.buf);
182
147
- while (!res && (e = readdir(dir))) {
148
- if (!strcmp(".", e->d_name) || !strcmp("..", e->d_name))
149
- continue;
183
+ while (!res && (e = readdir_skip_dot_and_dotdot(dir))) {
184
+ struct strbuf abspath = STRBUF_INIT;
185
+ unsigned char dtype;
186
+
187
+ strbuf_add_absolute_path(&abspath, at_root ? "." : path);
188
+ strbuf_addch(&abspath, '/');
189
+ dtype = get_dtype(e, &abspath);
190
191
strbuf_setlen(&buf, len);
192
strbuf_addstr(&buf, e->d_name);
193
154
- if (e->d_type == DT_REG)
194
+ if (dtype == DT_REG)
195
strvec_pushf(archiver_args, "--add-file=%s", buf.buf);
156
- else if (e->d_type != DT_DIR)
196
+ else if (dtype != DT_DIR)
197
warning(_("skipping '%s', which is neither file nor "
198
"directory"), buf.buf);
199
else if (recurse &&
200
add_directory_to_archiver(archiver_args,
201
buf.buf, recurse) < 0)
202
res = -1;
203
+
204
+ strbuf_release(&abspath);
205
}
206
207
closedir(dir);
t/t0092-diagnose.sh
+12
@@ -28,12 +28,23 @@ test_expect_success UNZIP 'creates diagnostics zip archive' '
28
! "$GIT_UNZIP" -l "$zip_path" | grep ".git/"
29
'
30
31
+test_expect_success UNZIP 'counts loose objects' '
32
+ test_commit A &&
33
+
34
+ # After committing, should have non-zero loose objects
35
+ git diagnose -o test-count -s 1 >out &&
36
+ zip_path=test-count/git-diagnostics-1.zip &&
37
+ "$GIT_UNZIP" -p "$zip_path" objects-local.txt >out &&
38
+ grep "^Total: [1-9][0-9]* loose objects" out
39
+'
40
+
41
test_expect_success UNZIP '--mode=stats excludes .git dir contents' '
42
test_when_finished rm -rf report &&
43
44
git diagnose -o report -s test --mode=stats >out &&
45
46
# Includes pack quantity/size info
47
+ zip_path=report/git-diagnostics-test.zip &&
48
"$GIT_UNZIP" -p "$zip_path" packs-local.txt >out &&
49
grep ".git/objects" out &&
50
@@ -47,6 +58,7 @@ test_expect_success UNZIP '--mode=all includes .git dir contents' '
58
git diagnose -o report -s test --mode=all >out &&
59
60
# Includes pack quantity/size info
61
+ zip_path=report/git-diagnostics-test.zip &&
62
"$GIT_UNZIP" -p "$zip_path" packs-local.txt >out &&
63
grep ".git/objects" out &&
64