Larry D'Anna [off-list ref] writes:
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.
I would agree that the logic to redirect the output to nowhere may be the
easiest way out, but because the reason anybody sane would want to give -q
is to say "I don't care what the actual changes are, but I want to know if
there is any real quick" (otherwise the call would be "diff -w >/dev/null"),
shouldn't we at least be exiting the loop early when we see any difference?
quoted hunk
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 */
Style?
+ static FILE *devnull = NULL;
Would this cause one file descriptor to leak? Do we care?
+ if(!devnull) {
Style? if (!devnull)
+ devnull = fopen("/dev/null", "w");
+ if (!devnull)
+ die_errno("Could not open /dev/null");
+ }
+ options->file = devnull;
Would this cause the original "options->file" leak? Do we care?
+ 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
* Junio C Hamano (gitster@pobox.com) [100216 00:42]:
Larry D'Anna [off-list ref] writes:
quoted
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.
I would agree that the logic to redirect the output to nowhere may be the
easiest way out, but because the reason anybody sane would want to give -q
is to say "I don't care what the actual changes are, but I want to know if
there is any real quick" (otherwise the call would be "diff -w >/dev/null"),
shouldn't we at least be exiting the loop early when we see any difference?
quoted
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 */
Style?
quoted
+ static FILE *devnull = NULL;
Would this cause one file descriptor to leak? Do we care?
Originally I thought it would be best to just let one leak, because I didn't
know how much longer it would need to stick around. I didn't notice it's being
closed anyway a few lines down.
quoted
+ if(!devnull) {
Style? if (!devnull)
quoted
+ devnull = fopen("/dev/null", "w");
+ if (!devnull)
+ die_errno("Could not open /dev/null");
+ }
+ options->file = devnull;
Would this cause the original "options->file" leak? Do we care?
oops.
quoted
+ 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
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 | 23 +++++++++++++++++++++++
1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/diff.c b/diff.c
index 68def6c..2984c41 100644
--- a/diff.c
+++ b/diff.c
@@ -3522,6 +3522,29 @@ 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
+ */
+ if (options->close_file)
+ fclose(options->file);
+ options->file = fopen("/dev/null", "w");
+ if (!options->file)
+ die_errno("Could not open /dev/null");
+ options->close_file = 1;
+ 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 (options->found_changes)
+ break;
+ }
+ }
+
if (output_format & DIFF_FORMAT_PATCH) {
if (separator) {
putc(options->line_termination, options->file);--
1.7.0.rc2.40.g7d8aa