Re: [PATCH 1/3] ewah_read_mmap: bounds-check mmap reads

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 15, 2018, 19:44 UTC
Message-ID: <xmqqsh5nzvfs.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180615172656.GB3067@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 16 quoted lines

> On Fri, Jun 15, 2018 at 10:05:42AM -0700, Junio C Hamano wrote:
>
>> > -	memcpy(self->buffer, ptr, self->buffer_size * sizeof(eword_t));
>> > -	ptr += self->buffer_size * sizeof(eword_t);
>> 
>> 
>> > +	data_len = st_mult(self->buffer_size, sizeof(eword_t));
>> 
>> This is a faithful conversion from the original, but I somehow would
>> have appreciated if the latter were not sizeof(eword_t) but rather
>> sizeof(self->buffer_size[0]), especially as I wondered ...
>
> I actually thought about going the _other_ way. The sizeof(eword_t) is
> not something we can change, but is actually decided by the on-disk
> format.  So I wondered if this should be much more clearly "hey, this is
> 8 bytes". Possibly with an assert(sizeof(*self->buffer_size) == 8).

Hmph, that's a thought.