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

[PATCH v2 1/2] Documentation/technical/bitmap-format.txt: add missing position table

From
Taylor Blau <me@ttaylorr.com>
Date
Jun 14, 2024, 19:23 UTC
Message-ID
<a71ec05e5dc0c8c40e1cce14a7c5fe946437a24d.1718392943.git.me@ttaylorr.com>
In-Reply-To
<cover.1718392943.git.me@ttaylorr.com>

While investigating a benign Coverity warning on the new pseudo-merge implementation, I was struggling to understand the (paraphrased) below:

    ofs = index_end - 24 - (index->pseudo_merges.nr * sizeof(uint64_t));
    for (i = 0; i < index->pseudo_merges.nr; i++) {
            index->pseudo_merges.v[i].at = get_be64(ofs);
            ofs += sizeof(uint64_t);
    }

, in pack-bitmap.c::load_bitmap_header(). Looking at the documentation, the diagram describing the on-disk format (prior to this patch) suggested that the optional extended lookup table immediately preceded the trailing metadata portion.

If that were the case, that would make the above code from load_bitmap_header() incorrect, as we'd be blindly reading into the extended offset table.

But later on in the documentation there is a description of the pseudo-merge position table as immediately preceding the trailing metadata portion of the extension. And indeed, we do write the position table in pack-bitmap-write.c:

    /* write positions for all pseudo merges */
    for (i = 0; i < writer->pseudo_merges_nr; i++)
            hashwrite_be64(f, pseudo_merge_ofs[i]);
    hashwrite_be32(f, writer->pseudo_merges_nr);
    hashwrite_be32(f, kh_size(writer->pseudo_merge_commits));
    hashwrite_be64(f, table_start - start);
    hashwrite_be64(f, hashfile_total(f) - start + sizeof(uint64_t));

So this is purely a case of the diagram being out of sync with the textual description and actual implementation of the format specification.

Add the missing component back to the format diagram to avoid further confusion in this area.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/technical/bitmap-format.txt | 9 +++++++++
 1 file changed, 9 insertions(+)
diff --git a/Documentation/technical/bitmap-format.txt b/Documentation/technical/bitmap-format.txt
index ee7775a258..bfb0ec7beb 100644
--- a/Documentation/technical/bitmap-format.txt
+++ b/Documentation/technical/bitmap-format.txt
@@ -312,6 +312,15 @@ the end of a `.bitmap` file. The format is as follows:
 |                                           |
 +-------------------------------------------+
 |                                           |
+|  Pseudo-merge position table              |
+|  +----+----------+----------+----------+  |
+|  | N  | Offset 1 |   ....   | Offset N |  |
+|  +----+----------+----------+----------+  |
+|  |    |  8 bytes |   ....   |  8 bytes |  |
+|  +----+----------+----------+----------+  |
+|                                           |
++-------------------------------------------+
+|                                           |
 |  Pseudo-merge Metadata                    |
 |  +-----------------------------------+    |
 |  | # pseudo-merges (4 bytes)         |    |
-- 
2.45.0.33.g0a16399d14.dirty
Previous: Taylor BlauNext: Taylor Blau
Message 9 of 12 in “Documentation/technical/bitmap-format.txt: add missing position table”
  1. 1/2 Documentation/technical/bitmap-format.txt: add missing position tableTaylor Blau, Jun 6, 2024
  2. 2/2 pack-bitmap.c: ensure pseudo-merge offset reads are boundedTaylor Blau, Jun 6, 2024
  3. Junio C HamanoJun 6, 2024
  4. Taylor BlauJun 6, 2024
  5. Junio C HamanoJun 6, 2024
  6. Taylor BlauJun 6, 2024
  7. Junio C HamanoJun 14, 2024
  8. 0/2 pseudo-merge: various small fixesTaylor Blau, Jun 14, 2024
  9. 1/2 Documentation/technical/bitmap-format.txt: add missing position tableTaylor Blau, Jun 14, 2024
  10. 2/2 pack-bitmap.c: ensure pseudo-merge offset reads are boundedTaylor Blau, Jun 14, 2024
  11. Elijah NewrenJun 14, 2024
  12. Junio C HamanoJun 14, 2024

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.