Re: problem in unpack-trees.c

Hi,

On Mon, 4 Dec 2006, Junio C Hamano wrote:

Roman Zippel [off-list ref] writes:

quoted

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.