difftool: avoid strcpy

In order to checkout files, difftool reads "diff --raw" output and feeds the names to checkout_entry(). That function requires us to have a "struct cache_entry". And because that struct uses a FLEX_ARRAY for the name field, we have to actually copy in our new name. The current code allocates a single re-usable cache_entry that can hold a name up to PATH_MAX, and then copies filenames into it using strcpy(). But there's no guarantee that incoming names are smaller than PATH_MAX. They've come from "diff --raw" output which might be diffing between two trees (and hence we'd be subject to the PATH_MAX of some other system, or even none at all if they were created directly via "update-index"). We can fix this by using make_cache_entry() to create a correctly-sized cache_entry for each name. This incurs an extra allocation per file, but this is negligible compared to actually writing out the file contents. To make this simpler, we can push this procedure into a new helper function. Note that we can also get rid of the "len" variables for src_path and dst_path (and in fact we must, as the compiler complains that they are unused). Signed-off-by: Jeff King <peff@peff.net> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Mar 30, 2017 at 06:35 UTC 0730dd4ffb39358f30b1a956cd9182aed1958b47
1 file changed +15 -16
builtin/difftool.c
+15 -16
@@ -297,6 +297,19 @@ static char *get_symlink(const struct object_id *oid, const char *path)
297 return data;
298 }
299
300 +static int checkout_path(unsigned mode, struct object_id *oid,
301 + const char *path, const struct checkout *state)
302 +{
303 + struct cache_entry *ce;
304 + int ret;
305 +
306 + ce = make_cache_entry(mode, oid->hash, path, 0, 0);
307 + ret = checkout_entry(ce, state, NULL);
308 +
309 + free(ce);
310 + return ret;
311 +}
312 +
313 static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
314 int argc, const char **argv)
315 {
@@ -306,7 +319,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
319 struct strbuf ldir = STRBUF_INIT, rdir = STRBUF_INIT;
320 struct strbuf wtdir = STRBUF_INIT;
321 size_t ldir_len, rdir_len, wtdir_len;
309 - struct cache_entry *ce = xcalloc(1, sizeof(ce) + PATH_MAX + 1);
322 const char *workdir, *tmp;
323 int ret = 0, i;
324 FILE *fp;
@@ -377,7 +389,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
389 struct object_id loid, roid;
390 char status;
391 const char *src_path, *dst_path;
380 - size_t src_path_len, dst_path_len;
392
393 if (starts_with(info.buf, "::"))
394 die(N_("combined diff formats('-c' and '--cc') are "
@@ -390,17 +401,14 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
401 if (strbuf_getline_nul(&lpath, fp))
402 break;
403 src_path = lpath.buf;
393 - src_path_len = lpath.len;
404
405 i++;
406 if (status != 'C' && status != 'R') {
407 dst_path = src_path;
398 - dst_path_len = src_path_len;
408 } else {
409 if (strbuf_getline_nul(&rpath, fp))
410 break;
411 dst_path = rpath.buf;
403 - dst_path_len = rpath.len;
412 }
413
414 if (S_ISGITLINK(lmode) || S_ISGITLINK(rmode)) {
@@ -430,11 +438,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
438 }
439
440 if (lmode && status != 'C') {
433 - ce->ce_mode = lmode;
434 - oidcpy(&ce->oid, &loid);
435 - strcpy(ce->name, src_path);
436 - ce->ce_namelen = src_path_len;
437 - if (checkout_entry(ce, &lstate, NULL))
441 + if (checkout_path(lmode, &loid, src_path, &lstate))
442 return error("could not write '%s'", src_path);
443 }
444
@@ -451,11 +455,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
455 hashmap_add(&working_tree_dups, entry);
456
457 if (!use_wt_file(workdir, dst_path, &roid)) {
454 - ce->ce_mode = rmode;
455 - oidcpy(&ce->oid, &roid);
456 - strcpy(ce->name, dst_path);
457 - ce->ce_namelen = dst_path_len;
458 - if (checkout_entry(ce, &rstate, NULL))
458 + if (checkout_path(rmode, &roid, dst_path, &rstate))
459 return error("could not write '%s'",
460 dst_path);
461 } else if (!is_null_oid(&roid)) {
@@ -625,7 +625,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
625 exit_cleanup(tmpdir, rc);
626
627 finish:
628 - free(ce);
628 strbuf_release(&ldir);
629 strbuf_release(&rdir);
630 strbuf_release(&wtdir);