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

> Thomas Gummerer <t.gummerer@gmail.com> 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.