Pierre Habouzit wrote:
On Wed, Sep 19, 2007 at 06:37:31AM +0000, Andreas Ericsson wrote:
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.
Well, there is little point. write_name_quoted behaviour changes if
the last argument is \0 or non-\0 (see patch comment and quote.c code),
so it does not really matter to inline the "putchar" IMHO.
-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 ;-)
I'm anal when it comes to code: the rule of the least surprise should
apply, and consistency is fundamental. And it happens that diff_options
are always called `opt' in diff.c, except in that place (and it allows
to write the prototype of the function on one line).
Then perhaps a separate patch for this would have been prudent? I'm not
against the change per se and I understand the reasoning behind it, but
it seems to go against Documentation/SubmittingPatches (submit one change
at a time).
--
Andreas Ericsson andreas.ericsson@xxxxxx
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
