On Apr 22, 2014, at 16:17, Jeff King wrote:
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);
I suspect that will produce a warning about a cast increasing pointer
alignment in some cases.
So why do any uint8_t math in the first place?
Just guessing it started out as uint8_t math throughout then the
warning about increasing alignment showed up so that part got changed
but the save pointer offset part did not.
I think we could write it as:
eword_t *old = self->buffer;
... realloc ...
self->rlw = self->buffer + (self->rlw - old);
Yeah that seems good too.
I'm fine with your patch, though.
thanks.
I tend to gravitate towards the most minimal change that fixes the
issue when touching someone else's code especially if I'm not familiar
with it. There's also the minor issue of optimizing out the pointer
arithmetic implicit divide by sizeof(eword_t) for the subtract and the
implicit multiply by sizeof(eword_t) for the add -- I didn't check to
see if one of the alternatives is best -- the one you mention above
with the cast (keeping the uint8_t math) is probably guaranteed not to
have any implicit multiply, but there's that pesky warning to get rid
of. Of course those multiplies and divides should just end up being
shifts so not a big deal if they didn't get optimized out (the realloc
time should dwarf them into irrelevancy anyway).
--Kyle
--
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