From: Linus Torvalds <torvalds@osdl.org>
To: Nicolas Pitre <nico@cam.org>
Cc: Sergey Vlasov <vsu@altlinux.ru>, Junio C Hamano <junkio@cox.net>,
git@vger.kernel.org
Subject: Re: heads-up: git-index-pack in "next" is broken
Date: Tue, 17 Oct 2006 13:51:08 -0700 (PDT) [thread overview]
Message-ID: <Pine.LNX.4.64.0610171339030.3962@g5.osdl.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0610171615340.1971@xanadu.home>
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.
Quite frankly, this all makes me go "Eww..". The original pack-file (well,
v2) format was well-defined and had none of these issues. In contrast, the
new code in 'next' is just _ugly_.
And the thing about "ugly" is that it also tends to mean "fragile" and
"buggy" and "hard to extend later".
And maybe it's just me, but I consider unions to be bug-prone on their
own. The "master" branch has exactly two unions: the "grep_expr" structure
contains one (where the union member is clearly defined by the node type
in that structure), and object.c has a "union any_object" that _literally_
exists as purely an allocation size issue (ie it is used _only_ to
allocate the maximum size of any of the possible structures).
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.
Is this something you want to bet a big project on?
Linus
next prev parent reply other threads:[~2006-10-17 20:51 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-10-17 4:55 heads-up: git-index-pack in "next" is broken Junio C Hamano
2006-10-17 15:39 ` Nicolas Pitre
2006-10-17 16:07 ` Junio C Hamano
2006-10-17 17:00 ` Nicolas Pitre
2006-10-17 18:11 ` Junio C Hamano
2006-10-17 18:47 ` Nicolas Pitre
2006-10-17 19:36 ` Sergey Vlasov
2006-10-17 20:10 ` Junio C Hamano
2006-10-17 20:25 ` Nicolas Pitre
2006-10-17 20:23 ` Nicolas Pitre
2006-10-17 20:51 ` Linus Torvalds [this message]
2006-10-17 21:21 ` Nicolas Pitre
2006-10-17 21:46 ` Linus Torvalds
2006-10-18 0:20 ` Nicolas Pitre
2006-10-18 0:57 ` Linus Torvalds
2006-10-18 2:08 ` Nicolas Pitre
2006-10-18 3:12 ` Linus Torvalds
2006-10-18 6:09 ` Davide Libenzi
2006-10-18 14:56 ` Linus Torvalds
2006-10-18 16:17 ` Davide Libenzi
2006-10-18 16:52 ` Linus Torvalds
2006-10-18 21:21 ` Davide Libenzi
2006-10-18 21:48 ` Linus Torvalds
2006-10-18 22:34 ` Davide Libenzi
2006-10-18 1:30 ` Junio C Hamano
2006-10-18 2:23 ` Nicolas Pitre
2006-10-18 4:16 ` Junio C Hamano
2006-10-18 5:07 ` Junio C Hamano
2006-10-18 10:00 ` Johannes Schindelin
2006-10-18 13:13 ` Nicolas Pitre
2006-10-18 13:02 ` Nicolas Pitre
2006-10-17 21:54 ` Junio C Hamano
2006-10-18 1:38 ` Nicolas Pitre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.64.0610171339030.3962@g5.osdl.org \
--to=torvalds@osdl.org \
--cc=git@vger.kernel.org \
--cc=junkio@cox.net \
--cc=nico@cam.org \
--cc=vsu@altlinux.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).