Stefan Sperling <stsp@stsp.name> wrote:
> On Wed, Jun 08, 2022 at 12:06:21PM +0200, Omar Polo wrote:
> > my only complaint is that doing a got_opentemp per inspected commit is
> > quite a lot, maybe we can re-use the same fd and truncate it before
> > usage.
> 
> Like this?

yes!

it's even better than what i was thinking actually; ok for me

> diff 7c115ff11cb012b171cd245c340398bce925b6df 06651924351e8d3f70588651a87a27ee938418e7
> blob - 9c71b5d3bdb6aaff2b375c0955d89bb66bb9d325
> blob + 28ba9a46f202c773426d431a909c5dec6304d3ca
> --- got/got.c
> +++ got/got.c
> @@ -3776,10 +3776,9 @@ match_changed_paths(int *have_match, struct got_pathli
>  static const struct got_error *
>  match_patch(int *have_match, struct got_commit_object *commit,
>      struct got_object_id *id, const char *path, int diff_context,
> -    struct got_repository *repo, regex_t *regex)
> +    struct got_repository *repo, regex_t *regex, FILE *f)
>  {
>  	const struct got_error *err = NULL;
> -	FILE *f;
>  	char *line = NULL;
>  	size_t linesize = 0;
>  	ssize_t linelen;
> @@ -3787,9 +3786,9 @@ match_patch(int *have_match, struct got_commit_object 
>  
>  	*have_match = 0;
>  
> -	f = got_opentemp();
> -	if (f == NULL)
> -		return got_error_from_errno("got_opentemp");
> +	err = got_opentemp_truncate(f);
> +	if (err)
> +		return err;
>  
>  	err = print_patch(commit, id, path, diff_context, repo, f);
>  	if (err)
> @@ -3808,8 +3807,6 @@ match_patch(int *have_match, struct got_commit_object 
>  	}
>  done:
>  	free(line);
> -	if (fclose(f) == EOF && err == NULL)
> -		err = got_error_from_errno("fclose");
>  	return err;
>  }
>  
> @@ -4016,7 +4013,8 @@ print_commits(struct got_object_id *root_id, struct go
>      struct got_repository *repo, const char *path, int show_changed_paths,
>      int show_patch, const char *search_pattern, int diff_context, int limit,
>      int log_branches, int reverse_display_order,
> -    struct got_reflist_object_id_map *refs_idmap, int one_line)
> +    struct got_reflist_object_id_map *refs_idmap, int one_line,
> +    FILE *tmpfile)
>  {
>  	const struct got_error *err;
>  	struct got_commit_graph *graph;
> @@ -4079,7 +4077,8 @@ print_commits(struct got_object_id *root_id, struct go
>  				    &changed_paths, &regex);
>  			if (have_match == 0 && show_patch) {
>  				err = match_patch(&have_match, commit, id,
> -				    path, diff_context, repo, &regex);
> +				    path, diff_context, repo, &regex,
> +				    tmpfile);
>  				if (err)
>  					break;
>  			}
> @@ -4207,6 +4206,7 @@ cmd_log(int argc, char *argv[])
>  	const char *errstr;
>  	struct got_reflist_head refs;
>  	struct got_reflist_object_id_map *refs_idmap = NULL;
> +	FILE *tmpfile = NULL;
>  
>  	TAILQ_INIT(&refs);
>  
> @@ -4398,9 +4398,18 @@ cmd_log(int argc, char *argv[])
>  		worktree = NULL;
>  	}
>  
> +	if (search_pattern && show_patch) {
> +		tmpfile = got_opentemp();
> +		if (tmpfile == NULL) {
> +			error = got_error_from_errno("got_opentemp");
> +			goto done;
> +		}
> +	}
> +
>  	error = print_commits(start_id, end_id, repo, path ? path : "",
>  	    show_changed_paths, show_patch, search_pattern, diff_context,
> -	    limit, log_branches, reverse_display_order, refs_idmap, one_line);
> +	    limit, log_branches, reverse_display_order, refs_idmap, one_line,
> +	    tmpfile);
>  done:
>  	free(path);
>  	free(repo_path);
> @@ -4414,6 +4423,8 @@ done:
>  	}
>  	if (refs_idmap)
>  		got_reflist_object_id_map_free(refs_idmap);
> +	if (tmpfile && fclose(tmpfile) == EOF && error == NULL)
> +		error = got_error_from_errno("fclose");
>  	got_ref_list_free(&refs);
>  	return error;
>  }
> blob - 82d4fbdc322a75119d06855ff7c5d7879bcb9648
> blob + 5df60c352cb02c68869dfd77742c78669f368df6
> --- include/got_opentemp.h
> +++ include/got_opentemp.h
> @@ -37,3 +37,6 @@ const struct got_error *got_opentemp_named(char **, FI
>  
>  /* Like got_opentemp_named() but returns a file descriptor instead of a FILE. */
>  const struct got_error *got_opentemp_named_fd(char **, int *, const char *);
> +
> +/* Truncate a file. This is useful for re-using open temporary files. */
> +const struct got_error *got_opentemp_truncate(FILE *);
> blob - 1c12d8425dd6f7aa30fb805c6ff288d3b84161c0
> blob + 63194824b5e659ee7b68160c5975355726baf8d1
> --- lib/diff.c
> +++ lib/diff.c
> @@ -31,6 +31,7 @@
>  #include "got_path.h"
>  #include "got_cancel.h"
>  #include "got_worktree.h"
> +#include "got_opentemp.h"
>  
>  #include "got_lib_diff.h"
>  #include "got_lib_delta.h"
> @@ -52,18 +53,6 @@ add_line_offset(off_t **line_offsets, size_t *nlines, 
>  }
>  
>  static const struct got_error *
> -reset_file(FILE *f)
> -{
> -	if (fpurge(f) == EOF)
> -		return got_error_from_errno("fpurge");
> -	if (ftruncate(fileno(f), 0L) == -1)
> -		return got_error_from_errno("ftruncate");
> -	if (fseeko(f, 0L, SEEK_SET) == -1)
> -		return got_error_from_errno("fseeko");
> -	return NULL;
> -}
> -
> -static const struct got_error *
>  diff_blobs(off_t **line_offsets, size_t *nlines,
>      struct got_diffreg_result **resultp, struct got_blob_object *blob1,
>      struct got_blob_object *blob2, FILE *f1, FILE *f2,
> @@ -91,12 +80,12 @@ diff_blobs(off_t **line_offsets, size_t *nlines,
>  		*resultp = NULL;
>  
>  	if (f1) {
> -		err = reset_file(f1);
> +		err = got_opentemp_truncate(f1);
>  		if (err)
>  			goto done;
>  	}
>  	if (f2) {
> -		err = reset_file(f2);
> +		err = got_opentemp_truncate(f2);
>  		if (err)
>  			goto done;
>  	}
> blob - e998f5bc48549bbbbbffb5ff88ae18e3e21c8c6f
> blob + 4c4e3bbe901a58b588c927fbd65dd7620b43e919
> --- lib/opentemp.c
> +++ lib/opentemp.c
> @@ -113,3 +113,15 @@ got_opentemp_named_fd(char **path, int *outfd, const c
>  	*outfd = fd;
>  	return err;
>  }
> +
> +const struct got_error *
> +got_opentemp_truncate(FILE *f)
> +{
> +	if (fpurge(f) == EOF)
> +		return got_error_from_errno("fpurge");
> +	if (ftruncate(fileno(f), 0L) == -1)
> +		return got_error_from_errno("ftruncate");
> +	if (fseeko(f, 0L, SEEK_SET) == -1)
> +		return got_error_from_errno("fseeko");
> +	return NULL;
> +}