git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jacob Keller <jacob.keller@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>,
	Git mailing list <git@vger.kernel.org>, Jeff King <peff@peff.net>
Subject: Re: [RFC 2/3] refspec: make sure stack refspec_item variables are zeroed
Date: Mon, 17 Aug 2020 09:49:49 -0700	[thread overview]
Message-ID: <CA+P7+xoC8SOn+fOtzL1GqN0P+eB67-L19yUwYo6UNrYoYGh8Lw@mail.gmail.com> (raw)
In-Reply-To: <xmqqd03p44gr.fsf@gitster.c.googlers.com>

On Mon, Aug 17, 2020 at 9:33 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> 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.
>

Right. Today the unused fields happen to never get checked. But by
being uninitialized we risk unexpected behavior in case either new
fields are added or in case those fields start being checked by the
code using the structure.

> > 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.

  reply	other threads:[~2020-08-17 17:34 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-15  0:25 [RFC 1/3] refspec: fix documentation referring to refspec_item Jacob Keller
2020-08-15  0:25 ` [RFC 2/3] refspec: make sure stack refspec_item variables are zeroed Jacob Keller
2020-08-17 16:33   ` Junio C Hamano
2020-08-17 16:49     ` Jacob Keller [this message]
2020-08-15  0:25 ` [RFC 3/3] refspec: add support for negative refspecs Jacob Keller
2020-08-17 18:02   ` Jacob Keller
2020-08-17 23:43   ` Junio C Hamano
2020-08-18  0:04     ` Jacob Keller
2020-08-18 17:41       ` Jeff King
2020-08-20 23:59         ` Jacob Keller
2020-08-21  2:33           ` Jeff King
2020-08-21 16:19             ` Junio C Hamano
2020-08-21 16:28               ` Jacob Keller
2020-08-21 17:16         ` Jacob Keller
2020-08-21 17:26           ` Jacob Keller
2020-08-21 18:21             ` Jacob Keller
2020-08-21 18:59               ` Jeff King
2020-08-17 16:18 ` [RFC 1/3] refspec: fix documentation referring to refspec_item Junio C Hamano
2020-08-21 21:17   ` Jacob Keller
2020-08-21 21:41     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+P7+xoC8SOn+fOtzL1GqN0P+eB67-L19yUwYo6UNrYoYGh8Lw@mail.gmail.com \
    --to=jacob.keller@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob.e.keller@intel.com \
    --cc=peff@peff.net \
    --subject='Re: [RFC 2/3] refspec: make sure stack refspec_item variables are zeroed' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).