Thread (12 messages) flat view 12 messages, 4 authors, 2018-11-30

Re: [PATCH 2/2] format-patch: don't include --stat with --range-diff output

From: Junio C Hamano <hidden>
Date: 2018-11-24 04:17:57

Possibly related (same subject, not in this thread)

Junio C Hamano [off-list ref] writes:
Ævar Arnfjörð Bjarmason  [off-list ref] writes:
quoted
 	if (rev->rdiff1) {
+		struct diff_options opts;
+		memcpy(&opts, &rev->diffopt, sizeof(opts));
+		opts.output_format &= ~(DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY);
+
 		fprintf_ln(rev->diffopt.file, "%s", rev->rdiff_title);
 		show_range_diff(rev->rdiff1, rev->rdiff2,
-				rev->creation_factor, 1, &rev->diffopt);
+				rev->creation_factor, 1, &opts);
I am not quite convinced if this shallow copy is a safe thing to do.
Quite honestly at this late in the release cycle, as a band-aid, I'd
rather see a simpler revert than a change like this that we have to
worry about what happens if/when show_range_diff() _thinks_ it is
done with the opts and ends up discarding resources (e.g. "FILE *")
that are shared with rev->diffopt that would still have to be used
later.
Well, let's take it anyway as-is, at least for today, as I notice
show_range_diff() itself does another shallow copy, so we are not
making anything dramatically worse.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help