Re: [PATCH 1/8] sequencer: introduce new commands to reset the revision

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 19, 2018, 08:59 UTC
Message-ID: <CAPig+cQbG2s-LrAo9+7C7=dXifbWFJ3SzuNa-QePHDk7egK=jg@mail.gmail.com>
In-Reply-To: <8a91bf2184a3da4c0d5a13ba184813068e51f5c8.1516225925.git.johannes.schindelin@gmx.de>

On Thu, Jan 18, 2018 at 10:35 AM, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:

Show 18 quoted lines

> [...]
> This commit implements the commands to label, and to reset to, given
> revisions. The syntax is:
>
>         label <name>
>         reset <name>
> [...]
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> diff --git a/sequencer.c b/sequencer.c
> @@ -1919,6 +1936,139 @@ static int do_exec(const char *command_line)
> +static int safe_append(const char *filename, const char *fmt, ...)
> +{
> +       [...]
> +       if (commit_lock_file(&lock) < 0) {
> +               rollback_lock_file(&lock);
> +               return error(_("failed to finalize '%s'."), filename);

s/\.//

Show 14 quoted lines

> +       }
> +
> +       return 0;
> +}
> +
> +static int do_reset(const char *name, int len)
> +{
> +       [...]
> +       if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)
> +               return -1;
> +
> +       for (i = 0; i < len; i++)
> +               if (isspace(name[i]))
> +                       len = i;

What is the purpose of this loop? I could imagine that it's trying to strip all whitespace from the end of 'name', however, to do that it would iterate backward, not forward. (Or perhaps it's trying to truncate at the first space, but then it would need to invert the condition or use 'break'.) Am I missing something obvious?

> +       read_cache_unmerged();
> +       if (!fill_tree_descriptor(&desc, &oid)) {
> +               error(_("Failed to find tree of %s."), oid_to_hex(&oid));

s/Failed/failed/ s/\.//

Show 5 quoted lines

> +               rollback_lock_file(&lock);
> +               free((void *)desc.buffer);
> +               strbuf_release(&ref_name);
> +               return -1;
> +       }