pack-bitmap: do not use gcc packed attribute

The "__attribute__" flag may be a noop on some compilers.
That's OK as long as the code is correct without the
attribute, but in this case it is not. We would typically
end up with a struct that is 2 bytes too long due to struct
padding, breaking both reading and writing of bitmaps.

Instead of marshalling the data in a struct, let's just
provide helpers for reading and writing the appropriate
types. Besides being correct on all platforms, the result is
more efficient and simpler to read.

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/csum-file.h b/csum-file.h
index 9dedb038ea9c77a8fadac370e039f1672e948325..da612c59c1bccab839e0a91aa0f37ed292ce59a0 100644 (file)
--- a/csum-file.h
+++ b/csum-file.h
@@ -39,4 +39,15 @@ extern void sha1flush(struct sha1file *f);
 extern void crc32_begin(struct sha1file *);
 extern uint32_t crc32_end(struct sha1file *);
 
+static inline void sha1write_u8(struct sha1file *f, uint8_t data)
+{
+       sha1write(f, &data, sizeof(data));
+}
+
+static inline void sha1write_be32(struct sha1file *f, uint32_t data)
+{
+       data = htonl(data);
+       sha1write(f, &data, sizeof(data));
+}
+
 #endif

diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 1218befaf2afa2f0be4be8a582a1f85bab334ed2..5d353ad6a7ebd3ee236f590f84fca5ec32acd6b2 100644 (file)
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -473,7 +473,6 @@ static void write_selected_commits_v1(struct sha1file *f,
 
        for (i = 0; i < writer.selected_nr; ++i) {
                struct bitmapped_commit *stored = &writer.selected[i];
-               struct bitmap_disk_entry on_disk;
 
                int commit_pos =
                        sha1_pos(stored->commit->object.sha1, index, index_nr, sha1_access);
@@ -481,11 +480,10 @@ static void write_selected_commits_v1(struct sha1file *f,
                if (commit_pos < 0)
                        die("BUG: trying to write commit not in index");
 
-               on_disk.object_pos = htonl(commit_pos);
-               on_disk.xor_offset = stored->xor_offset;
-               on_disk.flags = stored->flags;
+               sha1write_be32(f, commit_pos);
+               sha1write_u8(f, stored->xor_offset);
+               sha1write_u8(f, stored->flags);
 
-               sha1write(f, &on_disk, sizeof(on_disk));
                dump_bitmap(f, stored->write_as);
        }
 }

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 91e41015316e8d5c166cdee92c453ec23a483dab..9f8b9098113dc189217c3f133857cff34645173d 100644 (file)
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -197,13 +197,24 @@ static struct stored_bitmap *store_bitmap(struct bitmap_index *index,
        return stored;
 }
 
+static inline uint32_t read_be32(const unsigned char *buffer, size_t *pos)
+{
+       uint32_t result = get_be32(buffer + *pos);
+       (*pos) += sizeof(result);
+       return result;
+}
+
+static inline uint8_t read_u8(const unsigned char *buffer, size_t *pos)
+{
+       return buffer[(*pos)++];
+}
+
 static int load_bitmap_entries_v1(struct bitmap_index *index)
 {
        static const size_t MAX_XOR_OFFSET = 160;
 
        uint32_t i;
        struct stored_bitmap **recent_bitmaps;
-       struct bitmap_disk_entry *entry;
 
        recent_bitmaps = xcalloc(MAX_XOR_OFFSET, sizeof(struct stored_bitmap));
 
@@ -214,15 +225,12 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)
                uint32_t commit_idx_pos;
                const unsigned char *sha1;
 
-               entry = (struct bitmap_disk_entry *)(index->map + index->map_pos);
-               index->map_pos += sizeof(struct bitmap_disk_entry);
+               commit_idx_pos = read_be32(index->map, &index->map_pos);
+               xor_offset = read_u8(index->map, &index->map_pos);
+               flags = read_u8(index->map, &index->map_pos);
 
-               commit_idx_pos = ntohl(entry->object_pos);
                sha1 = nth_packed_object_sha1(index->pack, commit_idx_pos);
 
-               xor_offset = (int)entry->xor_offset;
-               flags = (int)entry->flags;
-
                bitmap = read_bitmap_1(index);
                if (!bitmap)
                        return -1;

diff --git a/pack-bitmap.h b/pack-bitmap.h
index 8b7f4e9f0df2d50ffab1c0535d678c69fd7d6e94..487600b18c79be22c4c611fc4c3d29d817f1f304 100644 (file)
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -5,12 +5,6 @@
 #include "khash.h"
 #include "pack-objects.h"
 
-struct bitmap_disk_entry {
-       uint32_t object_pos;
-       uint8_t xor_offset;
-       uint8_t flags;
-} __attribute__((packed));
-
 struct bitmap_disk_header {
        char magic[4];
        uint16_t version;