Am 13.10.2013 17:05, schrieb Matthieu Moy: > Jens Lehmann <Jens.Lehmann@xxxxxx> writes: > >> static struct lock_file lock_file; >> +#define SUBMODULE_WITH_GITDIR ((const char *)1) > > I don't like very much hardcoded addresses like this. Are you 100% sure > address 1 will never be returned by xstrdup on any platform? The risk is > small if not negligible, but I'm unconfortable with this. Isn't there an > alternative (NULL, or empty string) that is guaranteed to never happen? All alternatives I could think of would require an extra array storing that information, which I dismissed for performance and memory footprint reasons (NULL is already taken for not being a submodule). I think 1 is one of the safest hard coded addresses as on sane systems accessing the zeropage will trigger a segfault. And if someday someone wants to free the memory I expect the special casing of 1 to be rather obvious. But I'm open to alternatives and would change that if people disagree. >> +test_expect_success 'git mv moves a submodule with a .git directory and .gitmodules' ' > > This doesn't seem to test the problem I was having (move a file, not a > submodule). Is this intentional? Yes. The first idea was to simply move the update_path_in_gitmodules() into the "if (submodule_gitfile[i])"-block, but that would have resulted in not updating .gitmodules for submodules with a .git directory, which I would consider a bug. So I thought this was worth an extra test case, while I wasn't sure testing mv of a regular file to not issue a warning is a very useful test case in submodule context. > In any case, this fixes my problem, thanks! Sure, glad to help and thanks for testing! -- 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