Nguyen Thai Ngoc Duy <pclouds@xxxxxxxxx> says, > On Thu, Oct 6, 2011 at 12:31 AM, Phil Hord <hordp@xxxxxxxxx> wrote: >> - if (!suffix[i] || chdir(used_path)) >> + if (!suffix[i]) >> + return NULL; >> + gitfile = read_gitfile(used_path) ; >> + if (gitfile) >> + strcpy(used_path, gitfile); >> + if (chdir(used_path)) >> return NULL; >> path = validated_path; >> } > > This is room for improvement, the patch is fine as it is now. We could > improve error reporting here. If .git file points to nowhere, we get > "not a repository-kind of message. Except daemon.c, enter_repo() > callers always die() if enter_repo() returns NULL. We could move the > die() part (with improved error message) into enter_repo(). > > We could update enter_repo(const char *, int) to enter_repo(const char > *, int, int gently). If gently is 1, we never die() nor report > anything (ie. what we're doing now). daemon.c will need this, the rest > of callers will be happy with gently = 0. I like that. It wasn't clear to me what the 'gently' moniker meant before, but now I understand it. It could easily apply to this function and the new is_gitfile() to help reduce code duplication. In a different patch, though. Phil -- 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