[PATCH 1/2] bugfix: segfault on git diff --output=/bad/path

Subsystems: the rest

DORMANTno replies

2 messages, 1 author, 2016-06-15 · open the first message on its own page

[PATCH 1/2] bugfix: segfault on git diff --output=/bad/path

From: Larry D'Anna <hidden>
Date: 2016-06-15 22:48:17

The return value from fopen wasn't being checked.

Signed-off-by: Larry D'Anna <redacted>
---
 diff.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/diff.c b/diff.c
index 381cc8d..68def6c 100644
--- a/diff.c
+++ b/diff.c
@@ -2893,6 +2893,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		;
 	else if (!prefixcmp(arg, "--output=")) {
 		options->file = fopen(arg + strlen("--output="), "w");
+		if (!options->file)
+			die_errno("Could not open '%s'", arg + strlen("--output="));
 		options->close_file = 1;
 	} else
 		return 0;
-- 
1.7.0.rc2.40.g7d8aa

[PATCH 2/2] bugfix: git diff --quiet -w never returns with exit status 1

From: Larry D'Anna <hidden>
Date: 2016-06-15 22:48:17

The problem: -w causes the flag DIFF_FROM_CONTENTS to be set, which causes
diff_flush to set the flag HAS_CHANGES based on options->found_changes, which is
set by diff_flush_patch (if there were any changes).  However, --quiet causes
diff_flush to never call diff_flush_patch, so options->found_changes is always 0.

The solution: In this situation, call diff_flush_patch with options->file set to
/dev/null.

Rationale: diff_flush_patch expects to write its output to options->file.
Adding a "silence" flag to diff_flush_patch and everything it calls would be
more invasive.

Signed-off-by: Larry D'Anna <redacted>
---
 diff.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/diff.c b/diff.c
index 68def6c..ff00816 100644
--- a/diff.c
+++ b/diff.c
@@ -3522,6 +3522,26 @@ void diff_flush(struct diff_options *options)
 		separator++;
 	}
 
+	if (output_format & DIFF_FORMAT_NO_OUTPUT &&
+	    DIFF_OPT_TST(options, EXIT_WITH_STATUS) &&
+	    DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
+		/* run diff_flush_patch for the exit status */
+		/* setting options->file to /dev/null should be safe, becaue we
+		   aren't supposed to produce any output anyways */
+		static FILE *devnull = NULL;
+		if(!devnull) {
+			devnull = fopen("/dev/null", "w");
+			if (!devnull)
+				die_errno("Could not open /dev/null");
+		}
+		options->file = devnull;
+		for (i = 0; i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			if (check_pair_status(p))
+				diff_flush_patch(p, options);
+		}
+	}
+
 	if (output_format & DIFF_FORMAT_PATCH) {
 		if (separator) {
 			putc(options->line_termination, options->file);
-- 
1.7.0.rc2.40.g7d8aa
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help