Re: [PATCH v4 4/7] worktree: expose function to retrieve worktree names
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- May 6, 2025, 16:08 UTC
- Message-ID
- <CAPig+cSCFZArGbAXe77H3__nUAt1BTwDRsM6V2emqaMvHdxVgg@mail.gmail.com>
- In-Reply-To
- <CAP8UFD24jwKjP6XdD_SSv7ukgyqewCThMd-wQSRsDk00X7btiQ@mail.gmail.com>
On Tue, May 6, 2025 at 4:23 AM Christian Couder <christian.couder@gmail.com> wrote:
Show 48 quoted lines
> On Mon, May 5, 2025 at 10:52 AM Patrick Steinhardt <ps@pks.im> wrote:
> > +int get_worktree_names(struct repository *repo, struct strvec *out)
> > +{
> > + worktrees_dir = repo_git_path(repo, "worktrees");
> > + dir = opendir(worktrees_dir);
> > + if (!dir) {
> > + if (errno == ENOENT) {
> > + ret = 0;
> > + goto out;
> > + }
> > +
> > + ret = -1;
> > + goto out;
> > + }
> > +
> > + while ((d = readdir_skip_dot_and_dotdot(dir)) != NULL)
> > + strvec_push(out, d->d_name);
> > +
> > + ret = 0;
> > +out:
> > + if (dir)
> > + closedir(dir);
> > + free(worktrees_dir);
> > + return ret;
> > +}
>
> Nit: this function seems to use "goto out" a bit too much for me. What
> about something like:
>
> int get_worktree_names(struct repository *repo, struct strvec *out)
> {
> int ret = 0;
> char *worktrees_dir = repo_git_path(repo, "worktrees");
> DIR *dir = opendir(worktrees_dir);
>
> if (dir) {
> struct dirent *d;
> while ((d = readdir_skip_dot_and_dotdot(dir)) != NULL)
> strvec_push(out, d->d_name);
> closedir(dir);
> } else {
> if (errno != ENOENT)
> ret = -1;
> }
>
> free(worktrees_dir);
> return ret;
> }I had the same response and made a slightly more concise proposal[*], though I'm not convinced that this function should exist at all.
[*]: https://lore.kernel.org/git/CAPig+cSDDbhGrym8j=PFKBCUxBQhZPzAHXGvKy-Z6POA4Ju3sw@mail.gmail.com/