setup_temporary_shallow: avoid using inactive tempfile
authorJeff King <peff@peff.net>
Tue, 5 Sep 2017 12:14:16 +0000 (08:14 -0400)
committerJunio C Hamano <gitster@pobox.com>
Wed, 6 Sep 2017 08:19:52 +0000 (17:19 +0900)
When there are no shallow entries to write, we skip creating
the tempfile entirely and try to return the empty string.

But we do so by calling get_tempfile_path() on the inactive
tempfile object. This will trigger an assertion that kills
the program. The bug was introduced by 6e122b449b
(setup_temporary_shallow(): use tempfile module,
2015-08-10). But nobody seems to have noticed since then
because we do not end up calling this function at all when
there are no shallow items. In other words, this code path
is completely unexercised.

Since the tempfile object is a static global, it _is_
possible that we call the function twice, writing out
shallow info the first time and then "reusing" our tempfile
object the second time. But:

1. It seems unlikely that this was the intent, as hitting
this code path would imply somebody clearing the
shallow_info list between calls.

And if somebody _did_ call the function multiple times
without clearing the shallow_info list, we'd hit a
different BUG for trying to reuse an already-active
tempfile.

2. I verified by code inspection that the function is only
called once per program. And also replacing this code
with a BUG() and running the test suite demonstrates
that it is not triggered there.

So we could probably just replace this with an assertion and
confirm that it's never called. However, the original intent
does seem to be that you _could_ call it when the
shallow_info is empty. And that's easy enough to do; since
the return value doesn't need to point to a writable buffer,
we can just return a string literal.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
shallow.c
index f5591e56dab67d6113deb8196c4a68c8af3f2ee0..29194b475a5e50d1490e0faa1c3a77534e0eb518 100644 (file)
--- a/shallow.c
+++ b/shallow.c
@@ -307,7 +307,7 @@ const char *setup_temporary_shallow(const struct oid_array *extra)
         * is_repository_shallow() sees empty string as "no shallow
         * file".
         */
-       return get_tempfile_path(&temporary_shallow);
+       return "";
 }
 
 void setup_alternate_shallow(struct lock_file *shallow_lock,