Re: [PATCH 03/19] diff.c: drop 'nofirst' from emit_line_0

[Brandon Williams <bmwill@google.com>]

On 05/13, Stefan Beller wrote:
> In 250f79930d (diff.c: split emit_line() from the first char and the rest
> of the line, 2009-09-14) we introduced the local variable 'nofirst' that
> indicates if we have no first sign character. With the given implementation
> we had to use an extra variable unlike reusing 'first' because the lines
> first character could be '\0'.
> 
> Change the meaning of the 'first' argument to not mean the first character
> of the line, but rather just containing the sign that is prepended to the
> line. Refactor emit_line to not include the lines first character, but pass
> the complete line as well as a '\0' sign, which now serves as an indication
> not to print a sign.
> 
> With this patch other callers hard code the sign (which are '+', '-',
> ' ' and '\\') such that we do not run into unexpectedly emitting an
> error-nous '\0'.
> 
> The audit of the caller revealed that the sign cannot be '\n' or '\r',
> so remove that condition for trailing newline or carriage return in the
> sign; the else part of the condition handles the len==0 perfectly,
> so we can drop the if/else construct.
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  diff.c | 40 +++++++++++++++++-----------------------
>  1 file changed, 17 insertions(+), 23 deletions(-)
> 
> diff --git a/diff.c b/diff.c
> index c2ed605cd0..4269b8dccf 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -517,33 +517,24 @@ static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
>  }
>  
>  static void emit_line_0(struct diff_options *o, const char *set, const char *reset,
> -			int first, const char *line, int len)
> +			int sign, const char *line, int len)
>  {
>  	int has_trailing_newline, has_trailing_carriage_return;
> -	int nofirst;
>  	FILE *file = o->file;
>  
>  	fputs(diff_line_prefix(o), file);
>  
> -	if (len == 0) {
> -		has_trailing_newline = (first == '\n');
> -		has_trailing_carriage_return = (!has_trailing_newline &&
> -						(first == '\r'));
> -		nofirst = has_trailing_newline || has_trailing_carriage_return;
> -	} else {
> -		has_trailing_newline = (len > 0 && line[len-1] == '\n');
> -		if (has_trailing_newline)
> -			len--;
> -		has_trailing_carriage_return = (len > 0 && line[len-1] == '\r');
> -		if (has_trailing_carriage_return)
> -			len--;
> -		nofirst = 0;
> -	}
> +	has_trailing_newline = (len > 0 && line[len-1] == '\n');
> +	if (has_trailing_newline)
> +		len--;
> +	has_trailing_carriage_return = (len > 0 && line[len-1] == '\r');
> +	if (has_trailing_carriage_return)
> +		len--;

Does the order of newline/carriage return always the same?

>  
> -	if (len || !nofirst) {
> +	if (len || sign) {
>  		fputs(set, file);
> -		if (!nofirst)
> -			fputc(first, file);
> +		if (sign)
> +			fputc(sign, file);
>  		fwrite(line, len, 1, file);
>  		fputs(reset, file);
>  	}
> @@ -556,7 +547,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
>  static void emit_line(struct diff_options *o, const char *set, const char *reset,
>  		      const char *line, int len)
>  {
> -	emit_line_0(o, set, reset, line[0], line+1, len-1);
> +	emit_line_0(o, set, reset, 0, line, len);
>  }
>  
>  static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
> @@ -4822,9 +4813,12 @@ void diff_flush(struct diff_options *options)
>  
>  	if (output_format & DIFF_FORMAT_PATCH) {
>  		if (separator) {
> -			fprintf(options->file, "%s%c",
> -				diff_line_prefix(options),
> -				options->line_termination);
> +			char term[2];
> +			term[0] = options->line_termination;
> +			term[1] = '\0';
> +
> +			emit_line(options, NULL, NULL,
> +				  term, 1);
>  			if (options->stat_sep) {
>  				/* attach patch instead of inline */
>  				fputs(options->stat_sep, options->file);
> -- 
> 2.13.0.18.g183880de0a
> 

-- 
Brandon Williams