Web lists-archives.com

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:

> > -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.

> > @@ -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?
> 
> > +		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