init: use strbufs to store paths

The init code predates strbufs, and uses PATH_MAX-sized buffers along with many manual checks on intermediate sizes (some of which make magic assumptions, such as that init will not create a path inside .git longer than 50 characters). We can simplify this greatly by using strbufs, which drops some hard-to-verify strcpy calls in favor of git_path_buf. While we're in the area, let's also convert existing calls to git_path to the safer git_path_buf (our existing calls were passed to pretty tame functions, and so were not a problem, but it's easy to be consistent and safe here). Note that we had an explicit test that "git init" rejects long template directories. This comes from 32d1776 (init: Do not segfault on big GIT_TEMPLATE_DIR environment variable, 2009-04-18). We can drop the test_must_fail here, as we now accept this and need only confirm that we don't segfault, which was the original point of the test. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 4, 2015 at 23:46 UTC 9c28390bda4bb86f48a3145417c1cb1892782c47
2 files changed +76 -100
builtin/init-db.c
+74 -98
@@ -36,10 +36,11 @@ static void safe_create_dir(const char *dir, int share)
36 die(_("Could not make %s writable by group"), dir);
37 }
38
39 -static void copy_templates_1(char *path, int baselen,
40 - char *template, int template_baselen,
39 +static void copy_templates_1(struct strbuf *path, struct strbuf *template,
40 DIR *dir)
41 {
42 + size_t path_baselen = path->len;
43 + size_t template_baselen = template->len;
44 struct dirent *de;
45
46 /* Note: if ".git/hooks" file exists in the repository being
@@ -49,77 +50,64 @@ static void copy_templates_1(char *path, int baselen,
50 * with the way the namespace under .git/ is organized, should
51 * be really carefully chosen.
52 */
52 - safe_create_dir(path, 1);
53 + safe_create_dir(path->buf, 1);
54 while ((de = readdir(dir)) != NULL) {
55 struct stat st_git, st_template;
55 - int namelen;
56 int exists = 0;
57
58 + strbuf_setlen(path, path_baselen);
59 + strbuf_setlen(template, template_baselen);
60 +
61 if (de->d_name[0] == '.')
62 continue;
60 - namelen = strlen(de->d_name);
61 - if ((PATH_MAX <= baselen + namelen) ||
62 - (PATH_MAX <= template_baselen + namelen))
63 - die(_("insanely long template name %s"), de->d_name);
64 - memcpy(path + baselen, de->d_name, namelen+1);
65 - memcpy(template + template_baselen, de->d_name, namelen+1);
66 - if (lstat(path, &st_git)) {
63 + strbuf_addstr(path, de->d_name);
64 + strbuf_addstr(template, de->d_name);
65 + if (lstat(path->buf, &st_git)) {
66 if (errno != ENOENT)
68 - die_errno(_("cannot stat '%s'"), path);
67 + die_errno(_("cannot stat '%s'"), path->buf);
68 }
69 else
70 exists = 1;
71
73 - if (lstat(template, &st_template))
74 - die_errno(_("cannot stat template '%s'"), template);
72 + if (lstat(template->buf, &st_template))
73 + die_errno(_("cannot stat template '%s'"), template->buf);
74
75 if (S_ISDIR(st_template.st_mode)) {
77 - DIR *subdir = opendir(template);
78 - int baselen_sub = baselen + namelen;
79 - int template_baselen_sub = template_baselen + namelen;
76 + DIR *subdir = opendir(template->buf);
77 if (!subdir)
81 - die_errno(_("cannot opendir '%s'"), template);
82 - path[baselen_sub++] =
83 - template[template_baselen_sub++] = '/';
84 - path[baselen_sub] =
85 - template[template_baselen_sub] = 0;
86 - copy_templates_1(path, baselen_sub,
87 - template, template_baselen_sub,
88 - subdir);
78 + die_errno(_("cannot opendir '%s'"), template->buf);
79 + strbuf_addch(path, '/');
80 + strbuf_addch(template, '/');
81 + copy_templates_1(path, template, subdir);
82 closedir(subdir);
83 }
84 else if (exists)
85 continue;
86 else if (S_ISLNK(st_template.st_mode)) {
94 - char lnk[256];
95 - int len;
96 - len = readlink(template, lnk, sizeof(lnk));
97 - if (len < 0)
98 - die_errno(_("cannot readlink '%s'"), template);
99 - if (sizeof(lnk) <= len)
100 - die(_("insanely long symlink %s"), template);
101 - lnk[len] = 0;
102 - if (symlink(lnk, path))
103 - die_errno(_("cannot symlink '%s' '%s'"), lnk, path);
87 + struct strbuf lnk = STRBUF_INIT;
88 + if (strbuf_readlink(&lnk, template->buf, 0) < 0)
89 + die_errno(_("cannot readlink '%s'"), template->buf);
90 + if (symlink(lnk.buf, path->buf))
91 + die_errno(_("cannot symlink '%s' '%s'"),
92 + lnk.buf, path->buf);
93 + strbuf_release(&lnk);
94 }
95 else if (S_ISREG(st_template.st_mode)) {
106 - if (copy_file(path, template, st_template.st_mode))
107 - die_errno(_("cannot copy '%s' to '%s'"), template,
108 - path);
96 + if (copy_file(path->buf, template->buf, st_template.st_mode))
97 + die_errno(_("cannot copy '%s' to '%s'"),
98 + template->buf, path->buf);
99 }
100 else
111 - error(_("ignoring template %s"), template);
101 + error(_("ignoring template %s"), template->buf);
102 }
103 }
104
105 static void copy_templates(const char *template_dir)
106 {
117 - char path[PATH_MAX];
118 - char template_path[PATH_MAX];
119 - int template_len;
107 + struct strbuf path = STRBUF_INIT;
108 + struct strbuf template_path = STRBUF_INIT;
109 + size_t template_len;
110 DIR *dir;
121 - const char *git_dir = get_git_dir();
122 - int len = strlen(git_dir);
111 char *to_free = NULL;
112
113 if (!template_dir)
@@ -132,26 +120,23 @@ static void copy_templates(const char *template_dir)
120 free(to_free);
121 return;
122 }
135 - template_len = strlen(template_dir);
136 - if (PATH_MAX <= (template_len+strlen("/config")))
137 - die(_("insanely long template path %s"), template_dir);
138 - strcpy(template_path, template_dir);
139 - if (template_path[template_len-1] != '/') {
140 - template_path[template_len++] = '/';
141 - template_path[template_len] = 0;
142 - }
143 - dir = opendir(template_path);
123 +
124 + strbuf_addstr(&template_path, template_dir);
125 + strbuf_complete(&template_path, '/');
126 + template_len = template_path.len;
127 +
128 + dir = opendir(template_path.buf);
129 if (!dir) {
130 warning(_("templates not found %s"), template_dir);
131 goto free_return;
132 }
133
134 /* Make sure that template is from the correct vintage */
150 - strcpy(template_path + template_len, "config");
135 + strbuf_addstr(&template_path, "config");
136 repository_format_version = 0;
137 git_config_from_file(check_repository_format_version,
153 - template_path, NULL);
154 - template_path[template_len] = 0;
138 + template_path.buf, NULL);
139 + strbuf_setlen(&template_path, template_len);
140
141 if (repository_format_version &&
142 repository_format_version != GIT_REPO_VERSION) {
@@ -162,17 +147,15 @@ static void copy_templates(const char *template_dir)
147 goto close_free_return;
148 }
149
165 - memcpy(path, git_dir, len);
166 - if (len && path[len - 1] != '/')
167 - path[len++] = '/';
168 - path[len] = 0;
169 - copy_templates_1(path, len,
170 - template_path, template_len,
171 - dir);
150 + strbuf_addstr(&path, get_git_dir());
151 + strbuf_complete(&path, '/');
152 + copy_templates_1(&path, &template_path, dir);
153 close_free_return:
154 closedir(dir);
155 free_return:
156 free(to_free);
157 + strbuf_release(&path);
158 + strbuf_release(&template_path);
159 }
160
161 static int git_init_db_config(const char *k, const char *v, void *cb)
@@ -199,28 +182,20 @@ static int needs_work_tree_config(const char *git_dir, const char *work_tree)
182
183 static int create_default_files(const char *template_path)
184 {
202 - const char *git_dir = get_git_dir();
203 - unsigned len = strlen(git_dir);
204 - static char path[PATH_MAX];
185 struct stat st1;
186 + struct strbuf buf = STRBUF_INIT;
187 + char *path;
188 char repo_version_string[10];
189 char junk[2];
190 int reinit;
191 int filemode;
192
211 - if (len > sizeof(path)-50)
212 - die(_("insane git directory %s"), git_dir);
213 - memcpy(path, git_dir, len);
214 -
215 - if (len && path[len-1] != '/')
216 - path[len++] = '/';
217 -
193 /*
194 * Create .git/refs/{heads,tags}
195 */
221 - safe_create_dir(git_path("refs"), 1);
222 - safe_create_dir(git_path("refs/heads"), 1);
223 - safe_create_dir(git_path("refs/tags"), 1);
196 + safe_create_dir(git_path_buf(&buf, "refs"), 1);
197 + safe_create_dir(git_path_buf(&buf, "refs/heads"), 1);
198 + safe_create_dir(git_path_buf(&buf, "refs/tags"), 1);
199
200 /* Just look for `init.templatedir` */
201 git_config(git_init_db_config, NULL);
@@ -244,16 +219,16 @@ static int create_default_files(const char *template_path)
219 */
220 if (shared_repository) {
221 adjust_shared_perm(get_git_dir());
247 - adjust_shared_perm(git_path("refs"));
248 - adjust_shared_perm(git_path("refs/heads"));
249 - adjust_shared_perm(git_path("refs/tags"));
222 + adjust_shared_perm(git_path_buf(&buf, "refs"));
223 + adjust_shared_perm(git_path_buf(&buf, "refs/heads"));
224 + adjust_shared_perm(git_path_buf(&buf, "refs/tags"));
225 }
226
227 /*
228 * Create the default symlink from ".git/HEAD" to the "master"
229 * branch, if it does not exist yet.
230 */
256 - strcpy(path + len, "HEAD");
231 + path = git_path_buf(&buf, "HEAD");
232 reinit = (!access(path, R_OK)
233 || readlink(path, junk, sizeof(junk)-1) != -1);
234 if (!reinit) {
@@ -266,10 +241,8 @@ static int create_default_files(const char *template_path)
241 "%d", GIT_REPO_VERSION);
242 git_config_set("core.repositoryformatversion", repo_version_string);
243
269 - path[len] = 0;
270 - strcpy(path + len, "config");
271 -
244 /* Check filemode trustability */
245 + path = git_path_buf(&buf, "config");
246 filemode = TEST_FILEMODE;
247 if (TEST_FILEMODE && !lstat(path, &st1)) {
248 struct stat st2;
@@ -290,14 +263,13 @@ static int create_default_files(const char *template_path)
263 /* allow template config file to override the default */
264 if (log_all_ref_updates == -1)
265 git_config_set("core.logallrefupdates", "true");
293 - if (needs_work_tree_config(git_dir, work_tree))
266 + if (needs_work_tree_config(get_git_dir(), work_tree))
267 git_config_set("core.worktree", work_tree);
268 }
269
270 if (!reinit) {
271 /* Check if symlink is supported in the work tree */
299 - path[len] = 0;
300 - strcpy(path + len, "tXXXXXX");
272 + path = git_path_buf(&buf, "tXXXXXX");
273 if (!close(xmkstemp(path)) &&
274 !unlink(path) &&
275 !symlink("testing", path) &&
@@ -308,31 +280,35 @@ static int create_default_files(const char *template_path)
280 git_config_set("core.symlinks", "false");
281
282 /* Check if the filesystem is case-insensitive */
311 - path[len] = 0;
312 - strcpy(path + len, "CoNfIg");
283 + path = git_path_buf(&buf, "CoNfIg");
284 if (!access(path, F_OK))
285 git_config_set("core.ignorecase", "true");
286 probe_utf8_pathname_composition();
287 }
288
289 + strbuf_release(&buf);
290 return reinit;
291 }
292
293 static void create_object_directory(void)
294 {
323 - const char *object_directory = get_object_directory();
324 - int len = strlen(object_directory);
325 - char *path = xmalloc(len + 40);
295 + struct strbuf path = STRBUF_INIT;
296 + size_t baselen;
297 +
298 + strbuf_addstr(&path, get_object_directory());
299 + baselen = path.len;
300 +
301 + safe_create_dir(path.buf, 1);
302
327 - memcpy(path, object_directory, len);
303 + strbuf_setlen(&path, baselen);
304 + strbuf_addstr(&path, "/pack");
305 + safe_create_dir(path.buf, 1);
306
329 - safe_create_dir(object_directory, 1);
330 - strcpy(path+len, "/pack");
331 - safe_create_dir(path, 1);
332 - strcpy(path+len, "/info");
333 - safe_create_dir(path, 1);
307 + strbuf_setlen(&path, baselen);
308 + strbuf_addstr(&path, "/info");
309 + safe_create_dir(path.buf, 1);
310
335 - free(path);
311 + strbuf_release(&path);
312 }
313
314 int set_git_dir_init(const char *git_dir, const char *real_git_dir,
t/t0001-init.sh
+2 -2
@@ -202,8 +202,8 @@ test_expect_success 'init honors global core.sharedRepository' '
202 x$(git config -f shared-honor-global/.git/config core.sharedRepository)
203 '
204
205 -test_expect_success 'init rejects insanely long --template' '
206 - test_must_fail git init --template=$(printf "x%09999dx" 1) test
205 +test_expect_success 'init allows insanely long --template' '
206 + git init --template=$(printf "x%09999dx" 1) test
207 '
208
209 test_expect_success 'init creates a new directory' '