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

[PATCH] diff: remove ternary operator evaluating always to true

From
Stefan Beller <stefanbeller@googlemail.com>
Date
Aug 8, 2013, 18:31 UTC
Message-ID
<1375986704-11441-1-git-send-email-stefanbeller@googlemail.com>

The line being changed is deep inside the function builtin_diff. The variable name_b, which is used to evaluate the ternary expression must evaluate to true at that position, hence the replacement with just name_b.

The name_b variable only occurs a few times in that lengthy function:
As a parameter to the function itself:
	static void builtin_diff(const char *name_a,
				 const char *name_b,
				...
The next occurrences are at:
	/* Never use a non-valid filename anywhere if at all possible */
	name_a = DIFF_FILE_VALID(one) ? name_a : name_b;
	name_b = DIFF_FILE_VALID(two) ? name_b : name_a;
	a_one = quote_two(a_prefix, name_a + (*name_a == '/'));
	b_two = quote_two(b_prefix, name_b + (*name_b == '/'));

In the last line of this block 'name_b' is dereferenced and compared to '/'. This would crash if name_b was NULL. Hence in the following code we can assume name_b being non-null.

The next occurrence is just as a function argument, which doesn't change
the memory, which name_b points to, so the assumption name_b being not
null still holds:
	emit_rewrite_diff(name_a, name_b, one, two,
				textconv_one, textconv_two, o);

The next occurrence would be the line of this patch. As name_b still must be not null, we can remove the ternary operator.

Inside the emit_rewrite_diff function there is a also a line
	ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
which was also simplified as there is also a dereference before the
ternary operator.
Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
---
 diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index 266112c..80f8439 100644
--- a/diff.c
+++ b/diff.c
@@ -669,7 +669,7 @@ static void emit_rewrite_diff(const char *name_a,
 	memset(&ecbdata, 0, sizeof(ecbdata));
 	ecbdata.color_diff = want_color(o->use_color);
 	ecbdata.found_changesp = &o->found_changes;
-	ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
+	ecbdata.ws_rule = whitespace_rule(name_b);
 	ecbdata.opt = o;
 	if (ecbdata.ws_rule & WS_BLANK_AT_EOF) {
 		mmfile_t mf1, mf2;
@@ -2372,7 +2372,7 @@ static void builtin_diff(const char *name_a,
 		ecbdata.label_path = lbl;
 		ecbdata.color_diff = want_color(o->use_color);
 		ecbdata.found_changesp = &o->found_changes;
-		ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
+		ecbdata.ws_rule = whitespace_rule(name_b);
 		if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
 			check_blank_at_eof(&mf1, &mf2, &ecbdata);
 		ecbdata.opt = o;
-- 
1.8.4.rc1.25.gd121ba2
Next: Jeff King
Message 1 of 6 in “diff: remove ternary operator evaluating always to true”
  1. diff: remove ternary operator evaluating always to trueStefan Beller, Aug 8, 2013
  2. Jeff KingAug 10, 2013
  3. Junio C HamanoAug 12, 2013
  4. Stefan BellerAug 12, 2013
  5. Stefan BellerAug 12, 2013
  6. Junio C HamanoAug 12, 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.