Re: [PATCH] mv: Fix spurious warning when moving a file in presence of submodules

[Jens Lehmann <Jens.Lehmann@web.de>]

Am 13.10.2013 17:05, schrieb Matthieu Moy:
> Jens Lehmann <Jens.Lehmann@web.de> 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!