Re: [PATCH 4/3] archive: specfile syntax change: "$Format:%PLCHLDR$" instead of just "%PLCHLDR"

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sep 6, 2007, 17:11 UTC
Message-ID: <Pine.LNX.4.64.0709061803590.28586@racer.site>
In-Reply-To: <46E028B9.2090908@lsrfire.ath.cx>

Hi,

On Thu, 6 Sep 2007, René Scharfe wrote:

> As suggested by Johannes, --pretty=format: placeholders in specfiles 
> need to be wrapped in $Format:...$ now.

Thanks.

Show 10 quoted lines

> diff --git a/builtin-archive.c b/builtin-archive.c
> index faccce3..a8a0f01 100644
> --- a/builtin-archive.c
> +++ b/builtin-archive.c
> @@ -81,14 +81,58 @@ static int run_remote_archiver(const char *remote, int argc,
>  	return !!rv;
>  }
>  
> +static void *format_specfile(const struct commit *commit, const char *format,
> +                             unsigned long *sizep)

Should this not be "char *buffer" instead of "const char *format"? Or even better: a "struct strbuf *"?

Show 9 quoted lines

> +{
> +	unsigned long len = *sizep, result_len = 0;
> +	const char *a = format;
> +	char *result = NULL;
> +
> +	for (;;) {
> +		const char *b, *c;
> +		char *fmt, *formatted = NULL;
> +		unsigned long a_len, fmt_len, formatted_len, allocated = 0;

Maybe initialise formatted_len, just to be on the safe side?

> +
> +		b = memchr(a, '$', len);
> +		if (!b || a + len < b + 9 || memcmp(b + 1, "Format:", 7))
> +			break;

Wouldn't memmem(buffer, len, "$Format:", 8) be better here?

A general comment: since you plan to output the result into a file anyway, it should be even easier to avoid realloc(), and do a print_formatted_specfile() instead of a format_specfile(), no?

Ciao, Dscho