Re: [PATCHv3 2/5] Learn to handle gitfiles in enter_repo
- From
Phil Hord <hordp@cisco.com>
- Date
- Oct 6, 2011, 19:16 UTC
- Message-ID
- <4E8DFE8F.3060408@cisco.com>
- In-Reply-To
- <CACsJy8BiZ7Ey95BOf4p-zwysyYwEY6WectRaj-GqnFvgDNTtZw@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> says,
Show 22 quoted lines
> On Thu, Oct 6, 2011 at 12:31 AM, Phil Hord <hordp@cisco.com> 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