Re: [PATCH/RFC v2 09/16] Read index-v5

[Junio C Hamano <gitster@xxxxxxxxx>]

Thomas Gummerer <t.gummerer@xxxxxxxxx> writes:

> +static struct directory_entry *read_directories_v5(unsigned int *dir_offset,
> +				unsigned int *dir_table_offset,
> +				void *mmap,
> +				int mmap_size)
> +{
> +	int i, ondisk_directory_size;
> +	uint32_t *filecrc, *beginning, *end;
> +	struct directory_entry *current = NULL;
> +	struct ondisk_directory_entry *disk_de;
> +	struct directory_entry *de;
> +	unsigned int data_len, len;
> +	char *name;
> +
> +	ondisk_directory_size = sizeof(disk_de->flags)
> +		+ sizeof(disk_de->foffset)
> +		+ sizeof(disk_de->cr)
> +		+ sizeof(disk_de->ncr)
> +		+ sizeof(disk_de->nsubtrees)
> +		+ sizeof(disk_de->nfiles)
> +		+ sizeof(disk_de->nentries)
> +		+ sizeof(disk_de->sha1);
> +	name = (char *)mmap + *dir_offset;
> +	beginning = mmap + *dir_table_offset;

Notice how you computed name with pointer arithmetic by first
casting mmap (which is "void *") and when computing beginning, you
forgot to cast mmap and attempted pointer arithmetic with "void *".
The latter does not work and breaks compilation.

The pointer-arith with "void *" is not limited to this function.

Please check the a band-aid (I wouldn't call it a fix-up) patch I
added on top of the series before queuing the topic to 'pu'; it is
primarily to illustrate the places I noticed that have this issue.

I do not necessarily suggest that the way the band-aid patch makes
it compile is the best approach.  It might be cleaner to use a saner
type like "char *" (or perhaps "const char *") as the type to point
at a piece of memory you read from the disk.  I haven't formed an
opinion.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html