Re: [PATCH 2/3] combine-diff: suppress a clang warning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



John Keeping <john@xxxxxxxxxxxxx> writes:

> 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++)
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]