git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 4, 2013, 00:24 UTC
Message-ID
<7vip696i3v.fsf@alter.siamese.dyndns.org>
In-Reply-To
<20130203231549.GV1342@serenity.lan>
John Keeping <john@keeping.me.uk> writes:
Show 12 quoted lines
>> 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.
>
> This seems like a sensible change.
>
> I generally like to get rid of the pointless warnings so that the useful
> ones can't hide in the noise.  Perhaps "CFLAGS += -Wno-string-plus-int"
> would be better for this particular warning, but when there's only one
> bit of code that triggers it, tweaking that seemed simpler.

Thanks for a sanity check. Ideally it should also have test cases to show "git diff --cc --raw blob1 blob2...blob$n" for n=4 and n=40 (or any two values clearly below and above the old hardcoded limit) behave sensibly, exposing the old breakage, which I'll leave as a LHF (low-hanging-fruit). Hint, hint...

-- >8 --
Subject: [PATCH] combine-diff: lift 32-way limit of combined diff

The "raw" format of combine-diff output is supposed to have as many colons as there are parents at the beginning, then blob modes for these parents, and then object names for these parents.

We weren't however prepared to handle a more than 32-way merge and did not show the correct number of colons in such a case.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 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++)
-- 
1.8.1.2.628.geb8a6d5
Previous: John KeepingNext: John Keeping
Message 11 of 20 in “Make Git compile warning-free with Clang”
  1. 0/3 Make Git compile warning-free with ClangJohn Keeping, Feb 3, 2013
  2. 1/3 fix clang -Wtautological-compare with unsigned enumJohn Keeping, Feb 3, 2013
  3. Jonathan NiederFeb 3, 2013
  4. 2/3 combine-diff: suppress a clang warningJohn Keeping, Feb 3, 2013
  5. Tay Ray ChuanFeb 3, 2013
  6. John KeepingFeb 3, 2013
  7. Junio C HamanoFeb 3, 2013
  8. John KeepingFeb 3, 2013
  9. Junio C HamanoFeb 3, 2013
  10. John KeepingFeb 3, 2013
  11. Junio C HamanoFeb 4, 2013
  12. t4038: add tests for "diff --cc --raw <trees>"John Keeping, Feb 5, 2013
  13. Junio C HamanoFeb 5, 2013
  14. t4038: add tests for "diff --cc --raw <trees>"John Keeping, Feb 5, 2013
  15. Junio C HamanoFeb 5, 2013
  16. Miles BaderFeb 7, 2013
  17. John KeepingFeb 7, 2013
  18. 3/3 builtin/apply: tighten (dis)similarity index parsingJohn Keeping, Feb 3, 2013
  19. Junio C HamanoFeb 3, 2013
  20. Antoine PelisseFeb 3, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.