On Thu, Jan 30, 2014 at 02:02:33PM -0800, Jonathan Nieder wrote:
> In an ideal world I would prefer to just rely on ntohll when it's
> decent (meaning that the '#if __BYTE_ORDER != __BIG_ENDIAN' block
> could be written as
>
> if (ntohll(1) != 1) {
> ...
> }
>
> or
>
> if (ntohll(1) == 1)
> ; /* Big endian. Nothing to do.
> else {
> ...
> }
>
> ). But compat/bswap.h already relies on knowing the endianness at
> preprocessing time so that wouldn't buy anything.
Yes, though it would simplify things because we are depending on ntohll
being defined, rather than some obscure macros.
> Another "in an ideal world" option: make the loop unconditional after
> checking that optimizers on big-endian systems realize it's a noop.
> In any event, in the real world your patch looks like the right thing
> to do.
I had the same thought when reading the original patch. The loop after
pre-processing on a big-endian system should look like:
{
size_t i;
for (i = 0; i < self->buffer_size; ++i)
self->buffer[i] = self->buffer[i];
}
It really seems like the sort of thing that any halfway decent compiler
should be able to turn into a noop. I'm OK to go that route, and if you
don't have a halfway decent compiler, tough cookies; git will waste your
precious nanoseconds doing a relatively small loop. If this loop
actually mattered, we would probably do better still to leave it in disk
order, and fix it up as-needed only when we look at a particular bitmap
(we do not typically need to look at all of them on disk).
-Peff
--
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