From: Pete Wyckoff <pw@padd.com>
Date: Mon, 02 Apr 2012 00:13:54 GMT
Subject: Re: [PATCH] fast-import: catch garbage after marks in from/merge
Message-ID: <20120402001354.GA12651@padd.com>
In-Reply-To: <20120401231259.GE20883@burratino>

jrnieder@gmail.com wrote on Sun, 01 Apr 2012 18:12 -0500:
> Hi Pete,
> 
> Pete Wyckoff wrote:
> 
> >     from :1M 100644 :103 hello.c
> >
> > It is missing a newline and should be:
> >
> >     from :1
> >     M 100644 :103 hello.c
> 
> Good idea; thanks.
> 
> I agree that this at least deserves a warning and probably should
> error out.
> 
> [...]
> > --- a/fast-import.c
> > +++ b/fast-import.c
> > @@ -2537,8 +2537,16 @@ static int parse_from(struct branch *b)
> >  		hashcpy(b->branch_tree.versions[0].sha1, t);
> >  		hashcpy(b->branch_tree.versions[1].sha1, t);
> >  	} else if (*from == ':') {
> > -		uintmax_t idnum = strtoumax(from + 1, NULL, 10);
> > -		struct object_entry *oe = find_mark(idnum);
> > +		char *eptr;
> > +		uintmax_t idnum = strtoumax(from + 1, &eptr, 10);
> > +		struct object_entry *oe;
> > +		if (eptr) {
> > +			for (; *eptr && isspace(*eptr); eptr++) ;
> > +			if (*eptr)
> > +				die("Garbage after mark: %s",
> 
> The implementation seems more complicated than it needs to be.  Why
> allow whitespace after the mark number?

Fear of breaking existing fast-import users that might happen
to have stray whitespace, or \r\n terminators.

Other similar fast-import are less forgiving, such as
parse_cat_blob.  Maybe we should generalize and enforce its
approach to parsing marks.

		-- Pete