Re: [PATCH] fast-import: catch garbage after marks in from/merge
- From
- Pete Wyckoff <pw@padd.com>
- Date
- Apr 2, 2012, 00:13 UTC
- Message-ID
- <20120402001354.GA12651@padd.com>
- In-Reply-To
- <20120401231259.GE20883@burratino>
jrnieder@gmail.com wrote on Sun, 01 Apr 2012 18:12 -0500:
Show 35 quoted lines
> 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