fetch-pack: disregard invalid pack lockfiles

9da69a6539 (fetch-pack: support more than one pack lockfile, 2020-06-10)
started to use a string_list for pack lockfile names instead of a single
string pointer.  It removed a NULL check from transport_unlock_pack() as
well, which is the function that eventually deletes these lockfiles and
releases their name strings.

index_pack_lockfile() can return NULL if it doesn't like the contents it
reads from the file descriptor passed to it.  unlink(2) is declared to
not accept NULL pointers (at least with glibc).  Undefined Behavior
Sanitizer together with Address Sanitizer detects a case where a NULL
lockfile name is passed to unlink(2) by transport_unlock_pack() in t1060
(make SANITIZE=address,undefined; cd t; ./t1060-object-corruption.sh).

Reinstate the NULL check to avoid undefined behavior, but put it right
at the source, so that the number of items in the string_list reflects
the number of valid lockfiles.

Signed-off-by: René Scharfe <l.s.r@web.de>
Reviewed-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/fetch-pack.c b/fetch-pack.c
index b10c4323155b77f2ef271564cef862147a880596..4625926cf07ea10f038876d1950bc1aa1df28b2a 100644 (file)
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -915,8 +915,9 @@ static int get_pack(struct fetch_pack_args *args,
        if (start_command(&cmd))
                die(_("fetch-pack: unable to fork off %s"), cmd_name);
        if (do_keep && pack_lockfiles) {
-               string_list_append_nodup(pack_lockfiles,
-                                        index_pack_lockfile(cmd.out));
+               char *pack_lockfile = index_pack_lockfile(cmd.out);
+               if (pack_lockfile)
+                       string_list_append_nodup(pack_lockfiles, pack_lockfile);
                close(cmd.out);
        }