Nguyễn Thái Ngọc Duy  <pclo...@gmail.com> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
> ---
>  wt-status.c | 52 +++++++++++++++++++++++++++++++---------------------
>  wt-status.h |  5 +++--
>  2 files changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index ef405d0..183aafe 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -970,7 +970,7 @@ static void show_bisect_in_progress(struct wt_status *s,
>   * Extract branch information from rebase/bisect
>   */
>  static void read_and_strip_branch(struct strbuf *sb,
> -                               const char **branch,
> +                               char **branch,
>                                 const char *path)
>  {
>       unsigned char sha1[20];
> @@ -994,52 +994,62 @@ static void read_and_strip_branch(struct strbuf *sb,
>               strbuf_addstr(sb, abbrev);
>               *branch = sb->buf;
>       } else if (!strcmp(sb->buf, "detached HEAD")) /* rebase */
> -             ;
> +             *branch = NULL;
>       else                    /* bisect */
>               *branch = sb->buf;
> +     if (*branch)
> +             *branch = xstrdup(*branch);
>  }

The reason why the original print_state() kept two strbufs in it was
because its use of the return value (in *branch) from this function
was private and it did not want to have to strdup anything.

With this change, I suspect that it is much saner to make this
function *not* take any external strbuf as input, because you are
always returning a piece of memory that belongs to the caller, or a
NULL.

In other words, with the new world order, wouldn't a saner function
signature be:

        static const char *read_and_strip_branch(const char **path);

after this patch?

Also I notice that the error-return cases of this function may be
wrong even before your patch.  Shouldn't it be doing *branch = NULL
(and 'return NULL' after the suggested change)?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to