From: "Todd C. Miller" <Todd.Miller@sudo.ws>
Subject: Re: diff_output_lines() performance fix
To: Stefan Sperling <stsp@stsp.name>
Cc: ori@eigenstate.org, gameoftrees@openbsd.org
Date: Wed, 07 Jul 2021 09:52:10 -0600

On Wed, 07 Jul 2021 16:10:03 +0200, Stefan Sperling wrote:

One minor comment inline.  Otherwise looks good to me.

 - todd

> diff c2cf8015b6da213404a3d87a14ff228d1ca99c50 c7c136ae4b43fc040bd55978735fb5e
> 81a7c400e
> blob - daf8b8cb069e52f434eb062cf50fbf0d99b33ecd
> blob + 46a9e0ca73dd9461540f0acb0aa537e3775489de
> --- lib/diff_output.c
> +++ lib/diff_output.c
> @@ -55,6 +55,8 @@ get_atom_byte(int *ch, struct diff_atom *atom, off_t o
>  	return 0;
>  }
>  
> +#define DIFF_OUTPUT_BUF_SIZE	512
> +
>  int
>  diff_output_lines(struct diff_output_info *outinfo, FILE *dest,
>  		  const char *prefix, struct diff_atom *start_atom,
> @@ -71,12 +73,16 @@ diff_output_lines(struct diff_output_info *outinfo, FI
>  
>  	foreach_diff_atom(atom, start_atom, count) {
>  		off_t outlen = 0;
> -		int i, ch;
> +		int i, ch, nbuf = 0;
>  		unsigned int len = atom->len;
> -		rc = fprintf(dest, "%s", prefix);
> -		if (rc < 0)
> -			return errno;
> -		outlen += rc;
> +		unsigned char buf[DIFF_OUTPUT_BUF_SIZE + 1 /* '\n' */];
> +		size_t n;
> +
> +		n = strlcpy(buf, prefix, sizeof(buf));
> +		if (n >= sizeof(buf))
> +			return ENOBUFS;

Should this be:
		if (n + 1 >= sizeof(buf))
			return ENOBUFS;

to leave room for that newline?  Alternately you could use
DIFF_OUTPUT_BUF_SIZE instead of sizeof(buf).

> +		nbuf += n;
> +
>  		if (len) {
>  			rc = get_atom_byte(&ch, atom, len - 1);
>  			if (rc)
> @@ -96,13 +102,18 @@ diff_output_lines(struct diff_output_info *outinfo, FI
>  			rc = get_atom_byte(&ch, atom, i);
>  			if (rc)
>  				return rc;
> -			rc = fprintf(dest, "%c", (unsigned char)ch);
> -			if (rc < 0)
> -				return errno;
> -			outlen += rc;
> +			if (nbuf >= DIFF_OUTPUT_BUF_SIZE) {
> +				rc = fwrite(buf, 1, nbuf, dest);
> +				if (rc != nbuf)
> +					return errno;
> +				outlen += rc;
> +				nbuf = 0;
> +			}
> +			buf[nbuf++] = ch;
>  		}
> -		rc = fprintf(dest, "\n");
> -		if (rc < 0)
> +		buf[nbuf++] = '\n';
> +		rc = fwrite(buf, 1, nbuf, dest);
> +		if (rc != nbuf)
>  			return errno;
>  		outlen += rc;
>  		if (outinfo) {
>