Re: [PATCH v2 1/4] wt-status: split wt_status_state parsing function out
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 6, 2013, 18:48 UTC
- Message-ID
- <7v4ngoqs8m.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1362572482-20570-2-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 30 quoted lines
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@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)?