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>
Karsten Blees committed
Nov 27, 2014 at 00:24 UTC
b5007211b6582fc38647ff695b5ac51541ea9de8
4 files changed
+29
-18
csum-file.h
+11
@@ -39,4 +39,15 @@ extern void sha1flush(struct sha1file *f);
39
extern void crc32_begin(struct sha1file *);
40
extern uint32_t crc32_end(struct sha1file *);
41
42
+static inline void sha1write_u8(struct sha1file *f, uint8_t data)
43
+{
44
+ sha1write(f, &data, sizeof(data));
45
+}
46
+
47
+static inline void sha1write_be32(struct sha1file *f, uint32_t data)
48
+{
49
+ data = htonl(data);
50
+ sha1write(f, &data, sizeof(data));
51
+}
52
+
53
#endif
pack-bitmap-write.c
+3
-5
@@ -473,7 +473,6 @@ static void write_selected_commits_v1(struct sha1file *f,
473
474
for (i = 0; i < writer.selected_nr; ++i) {
475
struct bitmapped_commit *stored = &writer.selected[i];
476
- struct bitmap_disk_entry on_disk;
476
477
int commit_pos =
478
sha1_pos(stored->commit->object.sha1, index, index_nr, sha1_access);
@@ -481,11 +480,10 @@ static void write_selected_commits_v1(struct sha1file *f,
480
if (commit_pos < 0)
481
die("BUG: trying to write commit not in index");
482
484
- on_disk.object_pos = htonl(commit_pos);
485
- on_disk.xor_offset = stored->xor_offset;
486
- on_disk.flags = stored->flags;
483
+ sha1write_be32(f, commit_pos);
484
+ sha1write_u8(f, stored->xor_offset);
485
+ sha1write_u8(f, stored->flags);
486
488
- sha1write(f, &on_disk, sizeof(on_disk));
487
dump_bitmap(f, stored->write_as);
488
}
489
}
pack-bitmap.c
+15
-7
@@ -197,13 +197,24 @@ static struct stored_bitmap *store_bitmap(struct bitmap_index *index,
197
return stored;
198
}
199
200
+static inline uint32_t read_be32(const unsigned char *buffer, size_t *pos)
201
+{
202
+ uint32_t result = get_be32(buffer + *pos);
203
+ (*pos) += sizeof(result);
204
+ return result;
205
+}
206
+
207
+static inline uint8_t read_u8(const unsigned char *buffer, size_t *pos)
208
+{
209
+ return buffer[(*pos)++];
210
+}
211
+
212
static int load_bitmap_entries_v1(struct bitmap_index *index)
213
{
214
static const size_t MAX_XOR_OFFSET = 160;
215
216
uint32_t i;
217
struct stored_bitmap **recent_bitmaps;
206
- struct bitmap_disk_entry *entry;
218
219
recent_bitmaps = xcalloc(MAX_XOR_OFFSET, sizeof(struct stored_bitmap));
220
@@ -214,15 +225,12 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)
225
uint32_t commit_idx_pos;
226
const unsigned char *sha1;
227
217
- entry = (struct bitmap_disk_entry *)(index->map + index->map_pos);
218
- index->map_pos += sizeof(struct bitmap_disk_entry);
228
+ commit_idx_pos = read_be32(index->map, &index->map_pos);
229
+ xor_offset = read_u8(index->map, &index->map_pos);
230
+ flags = read_u8(index->map, &index->map_pos);
231
220
- commit_idx_pos = ntohl(entry->object_pos);
232
sha1 = nth_packed_object_sha1(index->pack, commit_idx_pos);
233
223
- xor_offset = (int)entry->xor_offset;
224
- flags = (int)entry->flags;
225
-
234
bitmap = read_bitmap_1(index);
235
if (!bitmap)
236
return -1;
pack-bitmap.h
-6
@@ -5,12 +5,6 @@
5
#include "khash.h"
6
#include "pack-objects.h"
7
8
-struct bitmap_disk_entry {
9
- uint32_t object_pos;
10
- uint8_t xor_offset;
11
- uint8_t flags;
12
-} __attribute__((packed));
13
-
8
struct bitmap_disk_header {
9
char magic[4];
10
uint16_t version;