Re: [PATCHv4] fast-import: tighten parsing of datarefs

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 10, 2012, 21:40 UTC
Message-ID: <7vzkajnu3i.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120407225920.GA3948@padd.com>

Pete Wyckoff <pw@padd.com> writes:

Show 9 quoted lines

> The syntax for the use of mark references in fast-import
> demands either a SP (space) or LF (end-of-line) after
> a mark reference.  Fast-import does not complain when garbage
> appears after a mark reference in some cases.
> 
> Factor out parsing of mark references and complain if
> errant characters are found.  Also be a little more careful
> when parsing "inline" and SHA1s, complaining if extra
> characters appear or if the form of the dataref is unrecognized.

Show 11 quoted lines

> +static uintmax_t parse_mark_ref(const char *p, char **endptr)
> +{
> +	uintmax_t mark;
> +
> +	assert(*p == ':');
> +	++p;
> +	mark = strtoumax(p, endptr, 10);
> +	if (*endptr == p)
> +		die("No value after ':' in mark: %s", command_buf.buf);
> +	return mark;
> +}

Show 10 quoted lines

> +static uintmax_t parse_mark_ref_eol(const char *p)
> +{
> +...
> +}
> +
> +static uintmax_t parse_mark_ref_space(const char **p)
> +{
> +...
> +}
> +

The first helper looks sensible, but the two seemingly similar parse_mark_ref_WHATTOEXPECT() that have different interfaces are somewhat tasteless.

I wonder if the calling sites in file_change_m(), note_change_n(), parse_merge(), parse_cat_blob() and parse_treeish_dataref() can be made to share even more code by slight restructuring, though.