[PATCHv3 3/5] Add a common is_gitfile function

[PATCHv3 3/5] Add a common is_gitfile function

[Phil Hord]

There are at least two locations in the code that check for
gitfiles.  Each one is slightly different for pretty good
reasons.  Work towards a common API by abstracting the
"is this a gitfile" question into a single function.

Signed-off-by: Phil Hord <hordp@cisco.com>

diff --git a/builtin/clone.c b/builtin/clone.c
index 488f48e..5110399 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -120,13 +120,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)
 			return xstrdup(absolute_path(path));
 		} else if (S_ISREG(st.st_mode) && st.st_size > 8) {
 			/* Is it a "gitfile"? */
-			char signature[8];
-			int len, fd = open(path, O_RDONLY);
-			if (fd < 0)
-				continue;
-			len = read_in_full(fd, signature, 8);
-			close(fd);
-			if (len != 8 || strncmp(signature, "gitdir: ", 8))
+			if (!is_gitfile(path))
 				continue;
 			path = read_gitfile(path);
 			if (path) {
diff --git a/cache.h b/cache.h
index 7eeb8cf..61e1b0f 100644
--- a/cache.h
+++ b/cache.h
@@ -441,6 +441,7 @@ extern const char *get_git_work_tree(void);
 extern const char *read_gitfile(const char *path);
 extern const char *resolve_gitdir(const char *suspect);
 extern void set_git_work_tree(const char *tree);
+extern int is_gitfile(const char *path);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
diff --git a/setup.c b/setup.c
index 61c22e6..a3d5a41 100644
--- a/setup.c
+++ b/setup.c
@@ -358,7 +358,7 @@ const char *read_gitfile(const char *path)
 
 	if (stat(path, &st))
 		return NULL;
-	if (!S_ISREG(st.st_mode))
+	if (!is_gitfile(path))
 		return NULL;
 	fd = open(path, O_RDONLY);
 	if (fd < 0)
@@ -368,9 +368,6 @@ const char *read_gitfile(const char *path)
 	close(fd);
 	if (len != st.st_size)
 		die("Error reading %s", path);
-	buf[len] = '\0';
-	if (prefixcmp(buf, "gitdir: "))
-		die("Invalid gitfile format: %s", path);
 	while (buf[len - 1] == '\n' || buf[len - 1] == '\r')
 		len--;
 	if (len < 9)
@@ -397,6 +394,32 @@ const char *read_gitfile(const char *path)
 	return path;
 }
 
+/*
+ * See if the referenced file looks like a 'gitfile'.
+ * Does not try to determine if the referenced gitdir is actually valid.
+ */
+int is_gitfile(const char *path)
+{
+	struct stat st;
+	char buf[9];
+	int fd, len;
+	if (stat(path, &st))
+		return 0;
+	if (!S_ISREG(st.st_mode))
+		return 0;
+	if (st.st_size < 10 || st.st_size > PATH_MAX)
+		return 0;
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return 0;
+	len = read_in_full(fd, buf, sizeof(buf));
+	close(fd);
+	if (len != sizeof(buf))
+		return 0;
+	return !prefixcmp(buf, "gitdir: ");
+}
+
 static const char *setup_explicit_git_dir(const char *gitdirenv,
 					  char *cwd, int len,
 					  int *nongit_ok)
-- 
1.7.7.505.ga09f6

Re: [PATCHv3 3/5] Add a common is_gitfile function

[Nguyen Thai Ngoc Duy]

On Thu, Oct 6, 2011 at 12:33 AM, Phil Hord <hordp@cisco.com> wrote:
> diff --git a/setup.c b/setup.c
> index 61c22e6..a3d5a41 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -358,7 +358,7 @@ const char *read_gitfile(const char *path)
>
>        if (stat(path, &st))
>                return NULL;
> -       if (!S_ISREG(st.st_mode))
> +       if (!is_gitfile(path))
>                return NULL;
>        fd = open(path, O_RDONLY);
>        if (fd < 0)
> @@ -368,9 +368,6 @@ const char *read_gitfile(const char *path)
>        close(fd);
>        if (len != st.st_size)
>                die("Error reading %s", path);
> -       buf[len] = '\0';
> -       if (prefixcmp(buf, "gitdir: "))
> -               die("Invalid gitfile format: %s", path);
>        while (buf[len - 1] == '\n' || buf[len - 1] == '\r')
>                len--;
>        if (len < 9)

I think you are changing the behavior here. Currently if .git is a
file and not a valid gitfile, die() will be called and repo discovery
process stops. With your patch, it skips invalid .git files and moves
on.

I'm not saying it's good or bad, just pointing out.
-- 
Duy