@matheusbsbs / matheus-bagliano / commits / fcd12db6af

prefer git_pathdup to git_path in some possibly-dangerous cases

Because git_path uses a static buffer that is shared with calls to git_path, mkpath, etc, it can be dangerous to assign the result to a variable or pass it to a non-trivial function. The value may change unexpectedly due to other calls. None of the cases changed here has a known bug, but they're worth converting away from git_path because: 1. It's easy to use git_pathdup in these cases. 2. They use constructs (like assignment) that make it hard to tell whether they're safe or not. The extra malloc overhead should be trivial, as an allocation should be an order of magnitude cheaper than a system call (which we are clearly about to make, since we are constructing a filename). The real cost is that we must remember to free the result. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Aug 10, 2015 at 05:35 UTC fcd12db6af118b70b5c15cf5fdd6800eeecc370a

6 files changed +21 -11

builtin/fsck.c

+3 -1

@@ -243,13 +243,14 @@ static void check_unreachable_object(struct object *obj)

243 printf("dangling %s %s\n", typename(obj->type),

244 sha1_to_hex(obj->sha1));

245 if (write_lost_and_found) {

246 - const char *filename = git_path("lost-found/%s/%s",

246 + char *filename = git_pathdup("lost-found/%s/%s",

247 obj->type == OBJ_COMMIT ? "commit" : "other",

248 sha1_to_hex(obj->sha1));

249 FILE *f;

250

251 if (safe_create_leading_directories_const(filename)) {

252 error("Could not create lost-found");

253 + free(filename);

254 return;

255 }

256 if (!(f = fopen(filename, "w")))

@@ -262,6 +263,7 @@ static void check_unreachable_object(struct object *obj)

263 if (fclose(f))

264 die_errno("Could not finish '%s'",

265 filename);

266 + free(filename);

267 }

268 return;

269 }

fast-import.c

+3 -1

@@ -407,7 +407,7 @@ static void dump_marks_helper(FILE *, uintmax_t, struct mark_set *);

407

408 static void write_crash_report(const char *err)

409 {

410 - const char *loc = git_path("fast_import_crash_%"PRIuMAX, (uintmax_t) getpid());

410 + char *loc = git_pathdup("fast_import_crash_%"PRIuMAX, (uintmax_t) getpid());

411 FILE *rpt = fopen(loc, "w");

412 struct branch *b;

413 unsigned long lu;

@@ -415,6 +415,7 @@ static void write_crash_report(const char *err)

415

416 if (!rpt) {

417 error("can't write crash report %s: %s", loc, strerror(errno));

418 + free(loc);

419 return;

420 }

421

@@ -488,6 +489,7 @@ static void write_crash_report(const char *err)

489 fputs("-------------------\n", rpt);

490 fputs("END OF CRASH REPORT\n", rpt);

491 fclose(rpt);

492 + free(loc);

493 }

494

495 static void end_packfile(void);

http-backend.c

+2 -1

@@ -164,7 +164,7 @@ static void send_strbuf(const char *type, struct strbuf *buf)

164

165 static void send_local_file(const char *the_type, const char *name)

166 {

167 - const char *p = git_path("%s", name);

167 + char *p = git_pathdup("%s", name);

168 size_t buf_alloc = 8192;

169 char *buf = xmalloc(buf_alloc);

170 int fd;

@@ -191,6 +191,7 @@ static void send_local_file(const char *the_type, const char *name)

191 }

192 close(fd);

193 free(buf);

194 + free(p);

195 }

196

197 static void get_text_file(char *name)

notes-merge.c

+2 -1

@@ -295,7 +295,7 @@ static void write_buf_to_worktree(const unsigned char *obj,

295 const char *buf, unsigned long size)

296 {

297 int fd;

298 - const char *path = git_path(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));

298 + char *path = git_pathdup(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));

299 if (safe_create_leading_directories_const(path))

300 die_errno("unable to create directory for '%s'", path);

301 if (file_exists(path))

@@ -320,6 +320,7 @@ static void write_buf_to_worktree(const unsigned char *obj,

320 }

321

322 close(fd);

323 + free(path);

324 }

325

326 static void write_note_to_worktree(const unsigned char *obj,

refs.c

+8 -6

@@ -1288,12 +1288,12 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)

1288 */

1289 static struct packed_ref_cache *get_packed_ref_cache(struct ref_cache *refs)

1290 {

1291 - const char *packed_refs_file;

1291 + char *packed_refs_file;

1292

1293 if (*refs->name)

1294 - packed_refs_file = git_path_submodule(refs->name, "packed-refs");

1294 + packed_refs_file = git_pathdup_submodule(refs->name, "packed-refs");

1295 else

1296 - packed_refs_file = git_path("packed-refs");

1296 + packed_refs_file = git_pathdup("packed-refs");

1297

1298 if (refs->packed &&

1299 !stat_validity_check(&refs->packed->validity, packed_refs_file))

@@ -1312,6 +1312,7 @@ static struct packed_ref_cache *get_packed_ref_cache(struct ref_cache *refs)

1312 fclose(f);

1313 }

1314 }

1315 + free(packed_refs_file);

1316 return refs->packed;

1317 }

1318

@@ -1481,14 +1482,15 @@ static int resolve_gitlink_ref_recursive(struct ref_cache *refs,

1482 {

1483 int fd, len;

1484 char buffer[128], *p;

1484 - const char *path;

1485 + char *path;

1486

1487 if (recursion > MAXDEPTH || strlen(refname) > MAXREFLEN)

1488 return -1;

1489 path = *refs->name

1489 - ? git_path_submodule(refs->name, "%s", refname)

1490 - : git_path("%s", refname);

1490 + ? git_pathdup_submodule(refs->name, "%s", refname)

1491 + : git_pathdup("%s", refname);

1492 fd = open(path, O_RDONLY);

1493 + free(path);

1494 if (fd < 0)

1495 return resolve_gitlink_packed_ref(refs, refname, sha1);

1496

unpack-trees.c

+3 -1

@@ -1029,10 +1029,12 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options

1029 if (!core_apply_sparse_checkout || !o->update)

1030 o->skip_sparse_checkout = 1;

1031 if (!o->skip_sparse_checkout) {

1032 - if (add_excludes_from_file_to_list(git_path("info/sparse-checkout"), "", 0, &el, 0) < 0)

1032 + char *sparse = git_pathdup("info/sparse-checkout");

1033 + if (add_excludes_from_file_to_list(sparse, "", 0, &el, 0) < 0)

1034 o->skip_sparse_checkout = 1;

1035 else

1036 o->el = &el;

1037 + free(sparse);

1038 }

1039

1040 memset(&o->result, 0, sizeof(o->result));