Re: [PATCH v4 4/7] worktree: expose function to retrieve worktree names

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, May 6, 2025 at 4:23 AM Christian Couder
<christian.couder@gmail.com> wrote:
> 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/