On 10/1/2018 11:50 AM, Duy Nguyen wrote:
On Mon, Oct 1, 2018 at 3:46 PM Ben Peart <peartben@xxxxxxxxx> wrote:
@@ -1890,6 +1891,46 @@ static size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
static size_t read_eoie_extension(const char *mmap, size_t mmap_size);
static void write_eoie_extension(struct strbuf *sb, git_hash_ctx *eoie_context, size_t offset);
+struct load_index_extensions
+{
+#ifndef NO_PTHREADS
+ pthread_t pthread;
+#endif
+ struct index_state *istate;
+ const char *mmap;
+ size_t mmap_size;
+ unsigned long src_offset;
+};
+
+static void *load_index_extensions(void *_data)
+{
+ struct load_index_extensions *p = _data;
+ unsigned long src_offset = p->src_offset;
+
+ while (src_offset <= p->mmap_size - the_hash_algo->rawsz - 8) {
+ /* After an array of active_nr index entries,
+ * there can be arbitrary number of extended
+ * sections, each of which is prefixed with
+ * extension name (4-byte) and section length
+ * in 4-byte network byte order.
+ */
+ uint32_t extsize;
+ memcpy(&extsize, p->mmap + src_offset + 4, 4);
+ extsize = ntohl(extsize);
This could be get_be32() so that the next person will not need to do
another cleanup patch.
Good point, it was existing code so I focused on doing the minimal
change possible but I can clean it up since I'm touching it already.
+ if (read_index_extension(p->istate,
+ p->mmap + src_offset,
+ p->mmap + src_offset + 8,
+ extsize) < 0) {
This alignment is misleading because the conditions are aligned with
the code block below. If you can't align it with the '(', then just
add another tab.
Ditto. I'll make it:
uint32_t extsize = get_be32(p->mmap + src_offset + 4);
if (read_index_extension(p->istate,
p->mmap + src_offset,
p->mmap + src_offset + 8,
extsize) < 0) {
munmap((void *)p->mmap, p->mmap_size);
die(_("index file corrupt"));
}
+ munmap((void *)p->mmap, p->mmap_size);
This made me pause for a bit since we should not need to cast back to
void *. It turns out you need this because mmap pointer is const. But
you don't even need to munmap here. We're dying, the OS will clean
everything up.
I had the same thought about "we're about to die so why bother calling
munmap() here" but I decided rather than change it, I'd follow the
existing pattern just in case there was some platform/bug that required
it. I apparently doesn't cause harm as it's been that way a long time.
+ die(_("index file corrupt"));
+ }
+ src_offset += 8;
+ src_offset += extsize;
+ }
+
+ return NULL;
+}
