Re: [PATCH] Avoid segfault when passed malformed refspec

From: Sean <seanlkml@sympatico.ca>
Date: Feb 2, 2008, 01:26 UTC
Message-ID: <BAYC1-PASMTP11F54ED60A103C52F2406AAE310@CEZ.ICE>
In-Reply-To: <7vzluk6ugn.fsf@gitster.siamese.dyndns.org>

On Fri, 01 Feb 2008 17:03:04 -0800 Junio C Hamano <gitster@pobox.com> wrote:

Show 14 quoted lines

> Sean <seanlkml@sympatico.ca> writes:
> >
> > +		if (!rs[i].dst)
> > +			rs[i].dst = xstrdup("");
>
> I haven't followed the codepath carefully before responding, it
> feels like sweeping the breakage under the carpet, without
> fixing the real issue.
> 
> If the problem is a badly formatted input, shouldn't the code
> die loudly with diagnostic message, instead of pretending as if
> the user said something different (and sensible), especially
> without telling the user that that is what the code is doing?
>

Hey Junio,

You're probably right. It seemed like a reasonable fix at the time without having to understand the code too deeply. With the above patch, the code does complain to the user:

  $ git-fetch ../repo refs/heads/* :refs/heads/* 
  fatal: * refusing to create funny ref 'floop' locally

But surely a better error could be shown if fetch is made to squawk whenever a destination ref is omitted. I just wasn't confident enough in the code, or in knowing what refspec rules are universally applicable.

Sean