From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mon, 16 May 2016 00:40:19 GMT
Subject: Re: [PATCH 4/5] worktree: add "lock" command
Message-ID: <CAPig+cQ-weDTCnKuxapwaBp8W2T_VNuJSXPE_mRWPbjnO1krxQ@mail.gmail.com>
In-Reply-To: <20160510141729.23063-4-pclouds@gmail.com>

On Tue, May 10, 2016 at 10:17 AM, Nguyễn Thái Ngọc Duy
<pclouds@gmail.com> wrote:
> +static int lock_worktree(int ac, const char **av, const char *prefix)
> +{
> +       const char *reason = "", *old_reason;
> +       struct option options[] = {
> +               OPT_STRING(0, "reason", &reason, N_("string"),
> +                          N_("reason for locking")),
> +               OPT_END()
> +       };
> +       struct worktree **worktrees, *wt;
> +       struct strbuf dst = STRBUF_INIT;
> +
> +       ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
> +       if (ac != 1)
> +               usage_with_options(worktree_usage, options);
> +
> +       strbuf_addstr(&dst, prefix_filename(prefix,
> +                                           strlen(prefix),
> +                                           av[0]));
> +
> +       worktrees = get_worktrees();
> +       wt = find_worktree_by_path(worktrees, dst.buf);

Oh, I forgot to mention in my previous email that this is leaking
'strbuf dst'. It could be released right here at its point of final
use.

> +       if (!wt)
> +               die(_("'%s' is not a working directory"), av[0]);
> +       if (is_main_worktree(wt))
> +               die(_("'%s' is a main working directory"), av[0]);
> +
> +       old_reason = is_worktree_locked(wt);
> +       if (old_reason) {
> +               if (*old_reason)
> +                       die(_("already locked, reason: %s"), old_reason);
> +               die(_("already locked, no reason"));
> +       }
> +
> +       write_file(git_common_path("worktrees/%s/locked", wt->id),
> +                  "%s", reason);
> +       return 0;
> +}