From: Andreas Ericsson <ae@op5.se>
Date: Wed, 19 Sep 2007 06:37:31 GMT
Subject: Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.
Message-ID: <46F0C3AB.8010801@op5.se>
In-Reply-To: <20070918224122.2B55D344AB3@madism.org>

Pierre Habouzit wrote:
>  
> diff --git a/builtin-blame.c b/builtin-blame.c
> index e364b6c..16c0ca8 100644
> --- a/builtin-blame.c
> +++ b/builtin-blame.c
> @@ -1430,8 +1430,7 @@ static void get_commit_info(struct commit *commit,
>  static void write_filename_info(const char *path)
>  {
>  	printf("filename ");
> -	write_name_quoted(NULL, 0, path, 1, stdout);
> -	putchar('\n');
> +	write_name_quoted(path, stdout, '\n');
>  }
>  

This looks like a candidate for a macro. I'm not sure if gcc optimizes
sibling calls in void functions with -O2, and it doesn't inline without
-O3.

>  
> -static void diff_flush_raw(struct diff_filepair *p,
> -			   struct diff_options *options)
> +static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)

Parameter rename? I'd have thought the patch was big enough as it is ;-)


Other than that, the diffstat calls this a good patch, and given the fact that
all your previous series passed all tests, I assume this one does too.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231