Re: heads-up: git-index-pack in "next" is broken

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Linus Torvalds <torvalds@xxxxxxxx> writes:

> On Tue, 17 Oct 2006, Nicolas Pitre wrote:
>> On Tue, 17 Oct 2006, Sergey Vlasov wrote:
>> > 
>> > Yes, on x86_64 this is 24 because of 8-byte alignment for longs:
>> 
>> Ah bummer.  Then this is most likely the cause.  And here's a simple 
>> fix (Junio please confirm):
>
> Why do you use "unsigned long" in the first place?
>
> For some structure like this, it sounds positively wrong. Pack-files 
> should be architecture-neutral, which means that they shouldn't depend on 
> word-size, and they should be in some neutral byte-order.

This is an in-core structure if I am not mistaken, and is never
written out in the resulting pack file.  The program is reading
from .pack and building .idx that corresponds to it.  I agree
that it is ugly, but I do not hink using neutral byte-order in
this part of the processing buys us anything in this particular
case.

> In contrast, the new union introduced in "next" is just horrid. There's 
> not even any way to know which member to use, except apparently that it 
> expects that a SHA1 is never zero in the last 12 bytes. Which is probably 
> true, but still - that's some ugly stuff.

Again I agree with the ugliness objection, and I raised the
"expecting no collision" issue which Nico refuted later.  The
code is probably safe (not just safe in practice but safe in
theory as well), although that would not change the fact that it
is ugly X-<.

Nico, could we have an un-uglied version please?

-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]