git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 5/5] pack-objects: clamp negative depth to 0

From
Jeff King <peff@peff.net>
Date
May 1, 2021, 14:04 UTC
Message-ID
<YI1f8jKReuE3LXVH@coredump.intra.peff.net>
In-Reply-To
<YI1fbERSuS7Y3LKh@coredump.intra.peff.net>

A negative delta depth makes no sense, and the code is not prepared to handle it. If passed "--depth=-1" on the command line, then this line from break_delta_chains():

	cur->depth = (total_depth--) % (depth + 1);

triggers a divide-by-zero. This is undefined behavior according to the C standard, but on POSIX systems results in SIGFPE killing the process. This is certainly one way to inform the use that the command was invalid, but it's a bit friendlier to just treat it as "don't allow any deltas", which we already do for --depth=0.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/pack-objects.c      | 2 ++
 t/t5316-pack-delta-depth.sh | 7 +++++++
 2 files changed, 9 insertions(+)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ea7a5b3ba5..da5e0700f9 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3861,6 +3861,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	if (pack_to_stdout != !base_name || argc)
 		usage_with_options(pack_usage, pack_objects_options);
 
+	if (depth < 0)
+		depth = 0;
 	if (depth >= (1 << OE_DEPTH_BITS)) {
 		warning(_("delta chain depth %d is too deep, forcing %d"),
 			depth, (1 << OE_DEPTH_BITS) - 1);
diff --git a/t/t5316-pack-delta-depth.sh b/t/t5316-pack-delta-depth.sh
index 3e84df4137..759169d074 100755
--- a/t/t5316-pack-delta-depth.sh
+++ b/t/t5316-pack-delta-depth.sh
@@ -102,4 +102,11 @@ test_expect_success '--depth=0 disables deltas' '
 	test_cmp expect actual
 '
 
+test_expect_success 'negative depth disables deltas' '
+	pack=$(git pack-objects --all --depth=-1 </dev/null pack) &&
+	echo 0 >expect &&
+	max_chain pack-$pack.pack >actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.31.1.875.g5dccece0aa
Previous: Jeff KingNext: Derrick Stolee
Message 14 of 15 in “pack-objects: better handling of negative options”
  1. 0/5 pack-objects: better handling of negative optionsJeff King, May 1, 2021
  2. 1/5 t5300: modernize basic testsJeff King, May 1, 2021
  3. Junio C HamanoMay 3, 2021
  4. Jeff KingMay 3, 2021
  5. 2/5 t5300: check that we produced expected number of deltasJeff King, May 1, 2021
  6. 3/5 pack-objects: clamp negative window size to 0Jeff King, May 1, 2021
  7. Derrick StoleeMay 3, 2021
  8. Jeff KingMay 3, 2021
  9. René ScharfeMay 4, 2021
  10. Jeff KingMay 4, 2021
  11. Ævar Arnfjörð BjarmasonMay 5, 2021
  12. René ScharfeMay 5, 2021
  13. 4/5 t5316: check behavior of pack-objects --depth=0Jeff King, May 1, 2021
  14. 5/5 pack-objects: clamp negative depth to 0Jeff King, May 1, 2021
  15. Derrick StoleeMay 3, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.