read-cache.c: allocate index entries individually
The code to estimate the in-memory size of the index based on its on-disk representation is subtly wrong for certain architecture-dependent struct layouts. Instead of fixing it, replace the code to keep the index entries in a single large block of memory and allocate each entry separately instead. This is both simpler and more flexible, as individual entries can now be freed. Actually using that added flexibility is left for a later patch. Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx> Signed-off-by: Junio C Hamano <gitster@pobox.com>
René Scharfe committed
Oct 24, 2011 at 23:59 UTC
debed2a6291a29fd7b0e96f63fbf0142ed3280eb
2 files changed
+31
-51
cache.h
-1
@@ -316,7 +316,6 @@ struct index_state {
316
struct string_list *resolve_undo;
317
struct cache_tree *cache_tree;
318
struct cache_time timestamp;
319
- void *alloc;
319
unsigned name_hash_initialized : 1,
320
initialized : 1;
321
struct hash_table name_hash;
read-cache.c
+31
-50
@@ -1202,29 +1202,18 @@ int read_index(struct index_state *istate)
1202
return read_index_from(istate, get_index_file());
1203
}
1204
1205
-static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_entry *ce)
1205
+static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk)
1206
{
1207
+ struct cache_entry *ce;
1208
size_t len;
1209
const char *name;
1210
+ unsigned int flags;
1211
1210
- ce->ce_ctime.sec = ntohl(ondisk->ctime.sec);
1211
- ce->ce_mtime.sec = ntohl(ondisk->mtime.sec);
1212
- ce->ce_ctime.nsec = ntohl(ondisk->ctime.nsec);
1213
- ce->ce_mtime.nsec = ntohl(ondisk->mtime.nsec);
1214
- ce->ce_dev = ntohl(ondisk->dev);
1215
- ce->ce_ino = ntohl(ondisk->ino);
1216
- ce->ce_mode = ntohl(ondisk->mode);
1217
- ce->ce_uid = ntohl(ondisk->uid);
1218
- ce->ce_gid = ntohl(ondisk->gid);
1219
- ce->ce_size = ntohl(ondisk->size);
1212
/* On-disk flags are just 16 bits */
1221
- ce->ce_flags = ntohs(ondisk->flags);
1222
-
1223
- hashcpy(ce->sha1, ondisk->sha1);
1213
+ flags = ntohs(ondisk->flags);
1214
+ len = flags & CE_NAMEMASK;
1215
1225
- len = ce->ce_flags & CE_NAMEMASK;
1226
-
1227
- if (ce->ce_flags & CE_EXTENDED) {
1216
+ if (flags & CE_EXTENDED) {
1217
struct ondisk_cache_entry_extended *ondisk2;
1218
int extended_flags;
1219
ondisk2 = (struct ondisk_cache_entry_extended *)ondisk;
@@ -1232,7 +1221,7 @@ static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_en
1221
/* We do not yet understand any bit out of CE_EXTENDED_FLAGS */
1222
if (extended_flags & ~CE_EXTENDED_FLAGS)
1223
die("Unknown index entry format %08x", extended_flags);
1235
- ce->ce_flags |= extended_flags;
1224
+ flags |= extended_flags;
1225
name = ondisk2->name;
1226
}
1227
else
@@ -1240,25 +1229,26 @@ static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_en
1229
1230
if (len == CE_NAMEMASK)
1231
len = strlen(name);
1243
- /*
1244
- * NEEDSWORK: If the original index is crafted, this copy could
1245
- * go unchecked.
1246
- */
1247
- memcpy(ce->name, name, len + 1);
1248
-}
1232
1250
-static inline size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
1251
-{
1252
- long per_entry;
1233
+ ce = xmalloc(cache_entry_size(len));
1234
1254
- per_entry = sizeof(struct cache_entry) - sizeof(struct ondisk_cache_entry);
1235
+ ce->ce_ctime.sec = ntohl(ondisk->ctime.sec);
1236
+ ce->ce_mtime.sec = ntohl(ondisk->mtime.sec);
1237
+ ce->ce_ctime.nsec = ntohl(ondisk->ctime.nsec);
1238
+ ce->ce_mtime.nsec = ntohl(ondisk->mtime.nsec);
1239
+ ce->ce_dev = ntohl(ondisk->dev);
1240
+ ce->ce_ino = ntohl(ondisk->ino);
1241
+ ce->ce_mode = ntohl(ondisk->mode);
1242
+ ce->ce_uid = ntohl(ondisk->uid);
1243
+ ce->ce_gid = ntohl(ondisk->gid);
1244
+ ce->ce_size = ntohl(ondisk->size);
1245
+ ce->ce_flags = flags;
1246
1256
- /*
1257
- * Alignment can cause differences. This should be "alignof", but
1258
- * since that's a gcc'ism, just use the size of a pointer.
1259
- */
1260
- per_entry += sizeof(void *);
1261
- return ondisk_size + entries*per_entry;
1247
+ hashcpy(ce->sha1, ondisk->sha1);
1248
+
1249
+ memcpy(ce->name, name, len);
1250
+ ce->name[len] = '\0';
1251
+ return ce;
1252
}
1253
1254
/* remember to discard_cache() before reading a different cache! */
@@ -1266,7 +1256,7 @@ int read_index_from(struct index_state *istate, const char *path)
1256
{
1257
int fd, i;
1258
struct stat st;
1269
- unsigned long src_offset, dst_offset;
1259
+ unsigned long src_offset;
1260
struct cache_header *hdr;
1261
void *mmap;
1262
size_t mmap_size;
@@ -1305,29 +1295,18 @@ int read_index_from(struct index_state *istate, const char *path)
1295
istate->cache_nr = ntohl(hdr->hdr_entries);
1296
istate->cache_alloc = alloc_nr(istate->cache_nr);
1297
istate->cache = xcalloc(istate->cache_alloc, sizeof(struct cache_entry *));
1308
-
1309
- /*
1310
- * The disk format is actually larger than the in-memory format,
1311
- * due to space for nsec etc, so even though the in-memory one
1312
- * has room for a few more flags, we can allocate using the same
1313
- * index size
1314
- */
1315
- istate->alloc = xmalloc(estimate_cache_size(mmap_size, istate->cache_nr));
1298
istate->initialized = 1;
1299
1300
src_offset = sizeof(*hdr);
1319
- dst_offset = 0;
1301
for (i = 0; i < istate->cache_nr; i++) {
1302
struct ondisk_cache_entry *disk_ce;
1303
struct cache_entry *ce;
1304
1305
disk_ce = (struct ondisk_cache_entry *)((char *)mmap + src_offset);
1325
- ce = (struct cache_entry *)((char *)istate->alloc + dst_offset);
1326
- convert_from_disk(disk_ce, ce);
1306
+ ce = create_from_disk(disk_ce);
1307
set_index_entry(istate, i, ce);
1308
1309
src_offset += ondisk_ce_size(ce);
1330
- dst_offset += ce_size(ce);
1310
}
1311
istate->timestamp.sec = st.st_mtime;
1312
istate->timestamp.nsec = ST_MTIME_NSEC(st);
@@ -1361,11 +1340,15 @@ unmap:
1340
1341
int is_index_unborn(struct index_state *istate)
1342
{
1364
- return (!istate->cache_nr && !istate->alloc && !istate->timestamp.sec);
1343
+ return (!istate->cache_nr && !istate->timestamp.sec);
1344
}
1345
1346
int discard_index(struct index_state *istate)
1347
{
1348
+ int i;
1349
+
1350
+ for (i = 0; i < istate->cache_nr; i++)
1351
+ free(istate->cache[i]);
1352
resolve_undo_clear_index(istate);
1353
istate->cache_nr = 0;
1354
istate->cache_changed = 0;
@@ -1374,8 +1357,6 @@ int discard_index(struct index_state *istate)
1357
istate->name_hash_initialized = 0;
1358
free_hash(&istate->name_hash);
1359
cache_tree_free(&(istate->cache_tree));
1377
- free(istate->alloc);
1378
- istate->alloc = NULL;
1360
istate->initialized = 0;
1361
1362
/* no need to throw away allocated active_cache */