git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 17, 2020, 16:33 UTC
Message-ID
<xmqqd03p44gr.fsf@gitster.c.googlers.com>
In-Reply-To
<20200815002509.2467645-2-jacob.e.keller@intel.com>
Jacob Keller <jacob.e.keller@intel.com> writes:
Show 30 quoted lines
> 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.

Show 11 quoted lines
> 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.

Previous: Jacob KellerNext: Jacob Keller
Message 3 of 20 in “[RFC 1/3] refspec: fix documentation referring to refspec_item”
  1. Jacob KellerAug 15, 2020
  2. 2/3 refspec: make sure stack refspec_item variables are zeroedJacob Keller, Aug 15, 2020
  3. Junio C HamanoAug 17, 2020
  4. Jacob KellerAug 17, 2020
  5. 3/3 refspec: add support for negative refspecsJacob Keller, Aug 15, 2020
  6. Jacob KellerAug 17, 2020
  7. Junio C HamanoAug 17, 2020
  8. Jacob KellerAug 18, 2020
  9. Jeff KingAug 18, 2020
  10. Jacob KellerAug 20, 2020
  11. Jeff KingAug 21, 2020
  12. Junio C HamanoAug 21, 2020
  13. Jacob KellerAug 21, 2020
  14. Jacob KellerAug 21, 2020
  15. Jacob KellerAug 21, 2020
  16. Jacob KellerAug 21, 2020
  17. Jeff KingAug 21, 2020
  18. Junio C HamanoAug 17, 2020
  19. Jacob KellerAug 21, 2020
  20. Junio C HamanoAug 21, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.