{"thread":{"id":"28586","subject":"[PATCHv3 3/5] Add a common is_gitfile function","startedAt":"2011-10-05T13:33:54Z","lastAt":"2011-10-06T02:59:12Z","messageCount":2,"participants":["Phil Hord","Nguyen Thai Ngoc Duy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"176919","messageId":"4E8C5CC2.4030505@cisco.com","threadId":"28586","inReplyTo":null,"subject":"[PATCHv3 3/5] Add a common is_gitfile function","fromName":"Phil Hord","fromEmail":"hordp@cisco.com","sentAt":"2011-10-05T13:33:54Z","receivedAt":"2011-10-05T13:33:54Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"There are at least two locations in the code that check for\ngitfiles.  Each one is slightly different for pretty good\nreasons.  Work towards a common API by abstracting the\n\"is this a gitfile\" question into a single function.\n\nSigned-off-by: Phil Hord <hordp@cisco.com>\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 488f48e..5110399 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -120,13 +120,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)\n \t\t\treturn xstrdup(absolute_path(path));\n \t\t} else if (S_ISREG(st.st_mode) && st.st_size > 8) {\n \t\t\t/* Is it a \"gitfile\"? */\n-\t\t\tchar signature[8];\n-\t\t\tint len, fd = open(path, O_RDONLY);\n-\t\t\tif (fd < 0)\n-\t\t\t\tcontinue;\n-\t\t\tlen = read_in_full(fd, signature, 8);\n-\t\t\tclose(fd);\n-\t\t\tif (len != 8 || strncmp(signature, \"gitdir: \", 8))\n+\t\t\tif (!is_gitfile(path))\n \t\t\t\tcontinue;\n \t\t\tpath = read_gitfile(path);\n \t\t\tif (path) {\ndiff --git a/cache.h b/cache.h\nindex 7eeb8cf..61e1b0f 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -441,6 +441,7 @@ extern const char *get_git_work_tree(void);\n extern const char *read_gitfile(const char *path);\n extern const char *resolve_gitdir(const char *suspect);\n extern void set_git_work_tree(const char *tree);\n+extern int is_gitfile(const char *path);\n \n #define ALTERNATE_DB_ENVIRONMENT \"GIT_ALTERNATE_OBJECT_DIRECTORIES\"\n \ndiff --git a/setup.c b/setup.c\nindex 61c22e6..a3d5a41 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -358,7 +358,7 @@ const char *read_gitfile(const char *path)\n \n \tif (stat(path, &st))\n \t\treturn NULL;\n-\tif (!S_ISREG(st.st_mode))\n+\tif (!is_gitfile(path))\n \t\treturn NULL;\n \tfd = open(path, O_RDONLY);\n \tif (fd < 0)\n@@ -368,9 +368,6 @@ const char *read_gitfile(const char *path)\n \tclose(fd);\n \tif (len != st.st_size)\n \t\tdie(\"Error reading %s\", path);\n-\tbuf[len] = '\\0';\n-\tif (prefixcmp(buf, \"gitdir: \"))\n-\t\tdie(\"Invalid gitfile format: %s\", path);\n \twhile (buf[len - 1] == '\\n' || buf[len - 1] == '\\r')\n \t\tlen--;\n \tif (len < 9)\n@@ -397,6 +394,32 @@ const char *read_gitfile(const char *path)\n \treturn path;\n }\n \n+/*\n+ * See if the referenced file looks like a 'gitfile'.\n+ * Does not try to determine if the referenced gitdir is actually valid.\n+ */\n+int is_gitfile(const char *path)\n+{\n+\tstruct stat st;\n+\tchar buf[9];\n+\tint fd, len;\n+\tif (stat(path, &st))\n+\t\treturn 0;\n+\tif (!S_ISREG(st.st_mode))\n+\t\treturn 0;\n+\tif (st.st_size < 10 || st.st_size > PATH_MAX)\n+\t\treturn 0;\n+\n+\tfd = open(path, O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn 0;\n+\tlen = read_in_full(fd, buf, sizeof(buf));\n+\tclose(fd);\n+\tif (len != sizeof(buf))\n+\t\treturn 0;\n+\treturn !prefixcmp(buf, \"gitdir: \");\n+}\n+\n static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t\t\t\t\t  char *cwd, int len,\n \t\t\t\t\t  int *nongit_ok)\n-- \n1.7.7.505.ga09f6\n"},{"id":"176996","messageId":"CACsJy8C1yHYn8b85ia5SB9UfRBrpMT=xYkSmN4EjOoKP3xx_6w@mail.gmail.com","threadId":"28586","inReplyTo":"4E8C5CC2.4030505@cisco.com","subject":"Re: [PATCHv3 3/5] Add a common is_gitfile function","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-06T02:59:12Z","receivedAt":"2011-10-06T02:59:12Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Oct 6, 2011 at 12:33 AM, Phil Hord <hordp@cisco.com> wrote:\n> diff --git a/setup.c b/setup.c\n> index 61c22e6..a3d5a41 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -358,7 +358,7 @@ const char *read_gitfile(const char *path)\n>\n>        if (stat(path, &st))\n>                return NULL;\n> -       if (!S_ISREG(st.st_mode))\n> +       if (!is_gitfile(path))\n>                return NULL;\n>        fd = open(path, O_RDONLY);\n>        if (fd < 0)\n> @@ -368,9 +368,6 @@ const char *read_gitfile(const char *path)\n>        close(fd);\n>        if (len != st.st_size)\n>                die(\"Error reading %s\", path);\n> -       buf[len] = '\\0';\n> -       if (prefixcmp(buf, \"gitdir: \"))\n> -               die(\"Invalid gitfile format: %s\", path);\n>        while (buf[len - 1] == '\\n' || buf[len - 1] == '\\r')\n>                len--;\n>        if (len < 9)\n\nI think you are changing the behavior here. Currently if .git is a\nfile and not a valid gitfile, die() will be called and repo discovery\nprocess stops. With your patch, it skips invalid .git files and moves\non.\n\nI'm not saying it's good or bad, just pointing out.\n-- \nDuy\n"}]}