Re: [PATCH 03/14] tree-diff: clear parent array in path_appendnew()

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> All of the other functions which allocate a combine_diff_path struct
> zero out the parent array, but this code path does not. There's no bug,
> since our caller will fill in most of the fields. But leaving the unused
> fields (like combine_diff_parent.path) uninitialized makes working with
> the struct more error-prone than it needs to be.

OK.  We however will still not use the array at all when we do not
need it, so it would be between accessing uninitialized bytes vs
accessing 0-bytes by mistake?  With my devil's advocate hat on, I
wonder if this would lead to more sloppy users saying "I am not
following the pointer; I am merely stopping when I see a NULL
pointer at the end of the array" or something silly like that
without checking the validity of the array itself (which presumably
can be inferred by inspecting some other member in the containing
struct, right?)".

> Let's just zero the parent field to be consistent with the
> combine_diff_path_new() allocator.

But I like the "let's be consistent" reasoning, so I wouldn't
complain ;-)

> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  tree-diff.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tree-diff.c b/tree-diff.c
> index d9237ffd9b..24f7b5912c 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -151,8 +151,6 @@ static int emit_diff_first_parent_only(struct diff_options *opt, struct combine_
>   *	process(p);
>   *	p = pprev;
>   *	; don't forget to free tail->next in the end
> - *
> - * p->parent[] remains uninitialized.
>   */
>  static struct combine_diff_path *path_appendnew(struct combine_diff_path *last,
>  	int nparent, const struct strbuf *base, const char *path, int pathlen,
> @@ -187,6 +185,8 @@ static struct combine_diff_path *path_appendnew(struct combine_diff_path *last,
>  	p->mode = mode;
>  	oidcpy(&p->oid, oid ? oid : null_oid());
>  
> +	memset(p->parent, 0, sizeof(p->parent[0]) * nparent);
> +
>  	return p;
>  }