Yes, is_git_directory() is much better. Thanks for the pointer.

I will submit a patch unless I hear more suggestions from others.

Uma



On Mon, Aug 29, 2016 at 4:15 PM, Junio C Hamano [off-list ref] wrote:

Uma Srinivasan [off-list ref] writes:

quoted

On Mon, Aug 29, 2016 at 2:13 PM, Uma Srinivasan [off-list ref] wrote:

quoted

Ok that makes sense. Thanks much.

Uma

With respect to my original problem with a corrupted .git directory
under the submodule directory, I am thinking of adding the following 4
lines marked with ### to is_submodule_modified() to detect the
corrupted dir and die quickly instead of forking several child
processes:

               strbuf_addf(&buf, "%s/.git", path);
               git_dir = read_gitfile(buf.buf);
               if (!git_dir) {
                 ### strbuf_addf(&head_ref, "%s/HEAD",buf.buf);
                 ### if (strbuf_read_file(&temp_ref, head_ref.buf,0) < 0) {
                          ### die("Corrupted .git dir in submodule %s", path);
                 ###}
                         git_dir = buf.buf;
              }

This fixes my issue but what do you think? Is this the right way to
fix it? Is there a better way?

I think we already have a helper function that does a lot better
than "does it have a file called HEAD" to ask "is this a git
directory?" and its name I think is "is_git_directory" (I know, we
are not imaginative when naming our functions).

As to the check makes sense in the context of this function, I am
not an expert to judge.  I'd expect Jens, Heiko and/or Stefan to
know better than I do.