Re: [RFC 2/3] refspec: make sure stack refspec_item variables are zeroed

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

Jacob Keller <jacob.e.keller@intel.com> writes:

> From: Jacob Keller <jacob.keller@gmail.com>
>
> A couple of functions that used struct refspec_item did not zero out the
> structure memory. This can result in unexpected behavior, especially if
> additional parameters are ever added to refspec_item in the future. Use
> memset to ensure that unset structure members are zero.
>
> It may make sense to convert most of these uses of struct refspec_item
> to use either struct initializers or refspec_item_init_or_die. However,
> other similar code uses memset. Converting all of these uses has been
> left as a future exercise.
>
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
>  builtin/remote.c | 1 +
>  transport.c      | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/builtin/remote.c b/builtin/remote.c
> index c8240e9fcd58..542f56e3878b 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -478,6 +478,7 @@ static int get_head_names(const struct ref *remote_refs, struct ref_states *stat
>  	struct ref *fetch_map = NULL, **fetch_map_tail = &fetch_map;
>  	struct refspec_item refspec;
>  
> +	memset(&refspec, 0, sizeof(refspec));
>  	refspec.force = 0;
>  	refspec.pattern = 1;
>  	refspec.src = refspec.dst = "refs/heads/*";

The original leaves .matching and .exact_sha1 bitfields
uninitialized.  As .pattern is set to true, .exact_sha1
that is not initialized does not make get_fetch_map()
misbehave, and .matching is not used there, so the code
currently happens to be OK, but futureproofing is always
good.

> diff --git a/transport.c b/transport.c
> index 2d4fd851dc0f..419be0b6ea4b 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -443,6 +443,7 @@ void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int v
>  	if (ref->status != REF_STATUS_OK && ref->status != REF_STATUS_UPTODATE)
>  		return;
>  
> +	memset(&rs, 0, sizeof(rs));
>  	rs.src = ref->name;
>  	rs.dst = NULL;

The original here passes the rs to remote_find_tracking() with only
its .src and .dst filled.  The function is to find, given a concrete
ref, where the refspec tells it to go on the other end of the
connection, so the code currently happend to be OK without all other
fields, but again, futureproofing is good.