On Tue, Apr 22, 2014 at 03:53:02PM -0700, Kyle J. McKay wrote:
> So I was trying to use pack.writebitmaps=true and all I got was core dumps.
Eek.
> The fix with a real subject line ;) is below. I think perhaps this should be
> picked up for the 2.0.0 release. (Patch is against master.)
Yes, this is definitely the sort of bugfix we want to see during the -rc
period (well, we would prefer not to see bugs at all, but if we must
have them, fixes are helpful).
> ---- >8 ----
> Subject: [PATCH] ewah_bitmap.c: do not assume size_t and eword_t are the same
> size
Thanks for a very well-written commit message. I think your fix makes
sense:
> - self->rlw = self->buffer + (rlw_offset / sizeof(size_t));
> + self->rlw = self->buffer + (rlw_offset / sizeof(eword_t));
We could also write it as:
self->rlw = (uint8_t *)self->buffer + rlw_offset;
but I do not think that is necessarily any more readable, especially
because we probably need to cast it like:
self->rlw = (eword_t *)((uint8_t *)self->buffer + rlw_offset);
Given that self->rlw is a pointer to eword_t, though, we can assume
rlw_offset is always going to be a multiple of sizeof(eword_t) anyway
(and if it is not, the division in the original is a big problem, but I
do not think that is the case). So why do any uint8_t math in the first
place? I think we could write it as:
eword_t *old = self->buffer;
... realloc ...
self->rlw = self->buffer + (self->rlw - old);
I'm fine with your patch, though.
I also poked through the rest of the bitmap code looking for similar
problems, but didn't find any. I do not think this was a systemic issue
with bad use of types; it was just a think-o that happened to work on
64-bit machines.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
