From af8b61e1d0bdaba47f7fa44468c53c8e6e38608a Mon Sep 17 00:00:00 2001 From: Martin Leitner-Ankerl Date: Wed, 15 Jul 2026 16:23:30 +0200 Subject: [PATCH] file_scan: run is_block_zeroed() on the block being hashed Ports markfasheh/duperemove#405 by Tobias Klausmann. With --skip-zeroes, process_blocks() ran is_block_zeroed() on buffer->buf + buffer->dl_offset - always block 0 - instead of block #i, so it skipped either every block or none depending only on whether the first block happened to be zero. Check the block actually being hashed (matching the offset already passed to process_block() just below). Adds an integration test that scans a two-block file with block hashing and --skip-zeroes and asserts which block survives, catching the bug in both directions (leading and trailing zero block). Co-authored-by: Tobias Klausmann Co-Authored-By: Claude Fable 5 --- file_scan.c | 2 +- tests/integration/test_skip_zeroes.py | 34 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 tests/integration/test_skip_zeroes.py diff --git a/file_scan.c b/file_scan.c index 72c8a9393176..2e71e3826eb3 100644 --- a/file_scan.c +++ b/file_scan.c @@ -1267,7 +1267,7 @@ static ssize_t process_blocks(struct scan_ctxt *ctxt, struct buffer *buffer, for (unsigned int i = 0; i < nb_blocks; i++) { if (!is_block_ignored(ctxt->fiemap, curr_file_off) && !(options.skip_zeroes && - is_block_zeroed(buffer->buf + buffer->dl_offset))) { + is_block_zeroed(buffer->buf + i * blocksize))) { ret = process_block(buffer->buf + i * blocksize, blocksize, curr_file_off, hashes); if (ret) diff --git a/tests/integration/test_skip_zeroes.py b/tests/integration/test_skip_zeroes.py new file mode 100644 index 000000000000..0d5cd0294d9f --- /dev/null +++ b/tests/integration/test_skip_zeroes.py @@ -0,0 +1,34 @@ +"""--skip-zeroes must test the block actually being hashed, not always the +first block of the buffer. + +Regression test for the bug fixed upstream in markfasheh/duperemove#405, where +is_block_zeroed() was run on `buffer->buf + buffer->dl_offset` (always block 0) +instead of block #i. That made --skip-zeroes either skip every block or skip +none, depending only on whether the first block happened to be zero. +""" + +import os +from harness import DuperemoveTest + +BS = 4096 + + +class SkipZeroesTest(DuperemoveTest): + def _hashed_offsets(self, *blocks): + """Scan a single file made of the given 4K blocks with block hashing + + --skip-zeroes, and return the loffs of the blocks that got hashed.""" + self.write("f", b"".join(blocks)) + self.scan(self.path("f"), "-b", str(BS), + "--dedupe-options=partial", "--skip-zeroes") + self.assertDmOk() + return sorted(r[0] for r in self.hf_query("select loff from blocks")) + + def test_trailing_zero_block_is_skipped(self): + # [random][zeros]: only block 0 should be hashed. + # The bug tested block 0 (random) for both -> hashed [0, BS]. + self.assertEqual([0], self._hashed_offsets(os.urandom(BS), b"\0" * BS)) + + def test_leading_zero_block_does_not_skip_the_rest(self): + # [zeros][random]: block 1 must still be hashed. + # The bug tested block 0 (zeros) for both -> hashed nothing. + self.assertEqual([BS], self._hashed_offsets(b"\0" * BS, os.urandom(BS)))