Re: [PATCH 2/5] interpret-trailers: add an option to show only the trailers

On Wed, Aug 09, 2017 at 11:35:27AM -0700, Jonathan Tan wrote:

quoted

-static void print_all(FILE *outfile, struct list_head *head, int trim_empty)
+static void print_all(FILE *outfile, struct list_head *head,
+		      struct process_trailer_options *opts)

This can be const, I think. (Same thing for patch 1.)

OK. We often leave these kinds of option structs as non-const because
they can sometimes grow to carry state between functions (e.g.,
diff_opt). But it's certainly const-able now, so we can let somebody
undo it later if they want.

quoted

@@ -897,9 +899,10 @@ static int process_input_file(FILE *outfile,
 	trailer_info_get(&info, str);
 
 	/* Print lines before the trailers as is */
-	fwrite(str, 1, info.trailer_start - str, outfile);
+	if (outfile)

Any reason why you expect outfile to possibly be NULL?

quoted

+		fwrite(str, 1, info.trailer_start - str, outfile);
 
-	if (!info.blank_line_before_trailer)
+	if (outfile && !info.blank_line_before_trailer)

Same comment here.

Because of this hunk from later in the file where we pass in NULL:

        /* Print the lines before the trailers */
-       trailer_end = process_input_file(outfile, sb.buf, &head);
+       trailer_end = process_input_file(opts->only_trailers ? NULL : outfile,
+                                        sb.buf, &head);

-Peff