Re: [PATCH 2/3] combine-diff: suppress a clang warning
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 3, 2013, 21:07 UTC
- Message-ID
- <7v8v7585sr.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20130203203150.GU1342@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
Show 5 quoted lines
> A quick search turned up the original thread where this feature was
> added to Clang [1]. It seems that it does find genuine bugs where
> people try to log values by doing:
>
> log("failed to handle error: " + errno);To be perfectly honest, anybody who writes such a code should be sent back to school before trying to touch out code ever again ;-). It is not even valid Python, Perl nor Java, I would think.
> Are you happy to change COLONS to a const char[] instead of a #define?
Happy? Not really.
It could be a good change for entirely different reason. We will save space if we ever need to use it in multiple places. But the entire "COLONS + offset" thing was a hack we did, knowing that it will break when we end up showing a muiti-way diff for more than 32 blobs.
If we were to be touching that area of code, I'd rather see a change to make it more robust against such a corner case. If it results in squelching misguided clang warnings against programmers who should not be writing in C, that is a nice side effect, but I loathe to see any change whose primary purpose is to squelch pointless warnings.
combine-diff.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c index bb1cc96..7f6187f 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -982,14 +982,10 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent, free(sline); } -#define COLONS "::::::::::::::::::::::::::::::::" - static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct rev_info *rev) { struct diff_options *opt = &rev->diffopt; - int i, offset; - const char *prefix; - int line_termination, inter_name_termination; + int line_termination, inter_name_termination, i; line_termination = opt->line_termination; inter_name_termination = '\t'; @@ -1000,17 +996,14 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re show_log(rev); if (opt->output_format & DIFF_FORMAT_RAW) { - offset = strlen(COLONS) - num_parent; - if (offset < 0) - offset = 0; - prefix = COLONS + offset; + /* As many colons as there are parents */ + for (i = 0; i < num_parent; i++) + putchar(':'); /* Show the modes */ - for (i = 0; i < num_parent; i++) { - printf("%s%06o", prefix, p->parent[i].mode); - prefix = " "; - } - printf("%s%06o", prefix, p->mode); + for (i = 0; i < num_parent; i++) + printf("%06o ", p->parent[i].mode); + printf("%06o", p->mode); /* Show sha1's */ for (i = 0; i < num_parent; i++)