Hi,
On Mon, 4 Dec 2006, Junio C Hamano wrote:
> Roman Zippel <zippel@xxxxxxxxxxxxxx> writes:
>
> > I looked into it and the problem is during the "git-read-tree --reset"
> > step and it seems that the local df_conflict_entry variable of
> > unpack_trees() survives past that function. If you check in
> > add_cache_entry() it's called with this variable and only because
> > verify_path() fails it's not added to the tree on the other archs, but on
> > m68k the data on the stack is a bit different and thus verify_path()
> > succeeds and the stack variable is added to the tree and later saved.
>
> I am very puzzled about this.
>
> You are correct that the address of the df_conflict_entry is
> assigned to "struct unpack_trees_options *o" in unpack_trees(),
> and add_cache_entry() are called from many places in the call
> chain that starts from that function. And these call sites do
> rely on the conflict_entry to have a NUL name to prevent
> add_cache_entry from adding the entry to the index. Which feels
> like a hack, but it should get the job done while it is running.
Ok, I see, I wasn't sure that this part was really intentional.
> On my x86-64 box with gcc 4 (i.e. "#define FLEX_ARRAY /* empty */"
> is used,
>
> #include "cache.h"
>
> int
> main(int ac, char **av)
> {
> printf("sz %zu\n", sizeof(struct cache_entry));
> printf("of %zu\n", offsetof(struct cache_entry, name));
> memset(&dfc, 0, sizeof(dfc));
> }
>
> size of "struct cache_entry" is 64 while the offset of name
> member is 62, so I am luckily getting two bytes of room for
> memset to fill and cause name[] to be properly NUL terminated.
> If the alignment requirement of the platform is smaller, we may
> be overstepping the struct when we access its name[] member.
Yes, on m68k both values are the same and thus name is not initialized.
Your patch should do the trick, I'll give it a try.
bye, Roman
-
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