[v7,2/7] read-cache: clean up casting and byte decoding

Commit Message

Ben Peart Oct. 1, 2018, 1:45 p.m. UTC

From: Ben Peart <[e-mail]>

This patch does a clean up pass to minimize the casting required to work
with the memory mapped index (mmap).

It also makes the decoding of network byte order more consistent by using
get_be32() where possible.

Signed-off-by: Ben Peart <[e-mail]>
---
 read-cache.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

Comments

Duy Nguyen Oct. 1, 2018, 3:10 p.m. UTC | #1

On Mon, Oct 1, 2018 at 3:46 PM Ben Peart <[e-mail]> wrote:
>
> From: Ben Peart <[e-mail]>
>
> This patch does a clean up pass to minimize the casting required to work
> with the memory mapped index (mmap).
>
> It also makes the decoding of network byte order more consistent by using
> get_be32() where possible.
>
> Signed-off-by: Ben Peart <[e-mail]>
> ---
>  read-cache.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/read-cache.c b/read-cache.c
> index 583a4fb1f8..6ba99e2c96 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1650,7 +1650,7 @@ int verify_index_checksum;
>  /* Allow fsck to force verification of the cache entry order. */
>  int verify_ce_order;
>
> -static int verify_hdr(struct cache_header *hdr, unsigned long size)
> +static int verify_hdr(const struct cache_header *hdr, unsigned long size)

OK more constness. Good.

>  {
>         git_hash_ctx c;
>         unsigned char hash[GIT_MAX_RAWSZ];
> @@ -1674,7 +1674,7 @@ static int verify_hdr(struct cache_header *hdr, unsigned long size)
>  }
>
>  static int read_index_extension(struct index_state *istate,
> -                               const char *ext, void *data, unsigned long sz)
> +                               const char *ext, const char *data, unsigned long sz)

But it's not clear why you need to change the data type from void * to
char * here. I guess all the consumer functions take 'const char *'
anyway, so it's best to use 'const char *'?

Not worth a reroll (to give a reason why you do this in the commit
message), unless there are other changes.

Patch

diff --git a/read-cache.c b/read-cache.c
index 583a4fb1f8..6ba99e2c96 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1650,7 +1650,7 @@  int verify_index_checksum;
 /* Allow fsck to force verification of the cache entry order. */
 int verify_ce_order;
 
-static int verify_hdr(struct cache_header *hdr, unsigned long size)
+static int verify_hdr(const struct cache_header *hdr, unsigned long size)
 {
 	git_hash_ctx c;
 	unsigned char hash[GIT_MAX_RAWSZ];
@@ -1674,7 +1674,7 @@  static int verify_hdr(struct cache_header *hdr, unsigned long size)
 }
 
 static int read_index_extension(struct index_state *istate,
-				const char *ext, void *data, unsigned long sz)
+				const char *ext, const char *data, unsigned long sz)
 {
 	switch (CACHE_EXT(ext)) {
 	case CACHE_EXT_TREE:
@@ -1889,8 +1889,8 @@  int do_read_index(struct index_state *istate, const char *path, int must_exist)
 	int fd, i;
 	struct stat st;
 	unsigned long src_offset;
-	struct cache_header *hdr;
-	void *mmap;
+	const struct cache_header *hdr;
+	const char *mmap;
 	size_t mmap_size;
 	const struct cache_entry *previous_ce = NULL;
 
@@ -1918,7 +1918,7 @@  int do_read_index(struct index_state *istate, const char *path, int must_exist)
 		die_errno("unable to map index file");
 	close(fd);
 
-	hdr = mmap;
+	hdr = (const struct cache_header *)mmap;
 	if (verify_hdr(hdr, mmap_size) < 0)
 		goto unmap;
 
@@ -1943,7 +1943,7 @@  int do_read_index(struct index_state *istate, const char *path, int must_exist)
 		struct cache_entry *ce;
 		unsigned long consumed;
 
-		disk_ce = (struct ondisk_cache_entry *)((char *)mmap + src_offset);
+		disk_ce = (struct ondisk_cache_entry *)(mmap + src_offset);
 		ce = create_from_disk(istate, disk_ce, &consumed, previous_ce);
 		set_index_entry(istate, i, ce);
 
@@ -1961,21 +1961,20 @@  int do_read_index(struct index_state *istate, const char *path, int must_exist)
 		 * in 4-byte network byte order.
 		 */
 		uint32_t extsize;
-		memcpy(&extsize, (char *)mmap + src_offset + 4, 4);
-		extsize = ntohl(extsize);
+		extsize = get_be32(mmap + src_offset + 4);
 		if (read_index_extension(istate,
-					 (const char *) mmap + src_offset,
-					 (char *) mmap + src_offset + 8,
+					 mmap + src_offset,
+					 mmap + src_offset + 8,
 					 extsize) < 0)
 			goto unmap;
 		src_offset += 8;
 		src_offset += extsize;
 	}
-	munmap(mmap, mmap_size);
+	munmap((void *)mmap, mmap_size);
 	return istate->cache_nr;
 
 unmap:
-	munmap(mmap, mmap_size);
+	munmap((void *)mmap, mmap_size);
 	die("index file corrupt");
 }