summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2017-09-05 12:14:16 (GMT)
committerJunio C Hamano <gitster@pobox.com>2017-09-06 08:19:52 (GMT)
commit0899013993cd5eb327bc1a40d7c629db37e5ea83 (patch)
tree2c24a45c64844f6c98623bbc400beb2b7d1f1713
parentc82c75b9518750a94e38c84fc89741a51b815014 (diff)
downloadgit-0899013993cd5eb327bc1a40d7c629db37e5ea83.zip
git-0899013993cd5eb327bc1a40d7c629db37e5ea83.tar.gz
git-0899013993cd5eb327bc1a40d7c629db37e5ea83.tar.bz2
setup_temporary_shallow: avoid using inactive tempfile
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>
-rw-r--r--shallow.c2
1 files changed, 1 insertions, 1 deletions
diff --git a/shallow.c b/shallow.c
index f5591e5..29194b4 100644
--- 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,