Download raw body.
move got_opentempfd() out of lib/diff.c again
On Tue, Jun 28, 2022 at 10:08:10PM +0200, Stefan Sperling wrote:
> This moves all opentempfd() calls out of lib/diff.c.
>
> There is a bug fix in here where we forgot to rewind a temporary
> file after truncating it. This results in ERR_PRIVSEP_LEN when
> we verify the size of an extracted blob (I saw this happen during
> 'got histedit').
>
> ok?
ok! this was a majour improvement and a lot of work. thanks!
>
> diff 7a13e6e501f6808283b956b84746cc9e8d2f1b25 c6ae84322fdabec2faa256572927c44d6d3c68ad
> commit - 7a13e6e501f6808283b956b84746cc9e8d2f1b25
> commit + c6ae84322fdabec2faa256572927c44d6d3c68ad
> blob - e4665492a3022c605cf1a1fa244f10c606b8b0b7
> blob + b07d18f8d9b4cc592948a9a6fdfe9d7504989ee3
> --- got/got.c
> +++ got/got.c
> @@ -3609,6 +3609,7 @@ diff_trees(struct got_object_id *tree_id1, struct got_
> struct got_tree_object *tree1 = NULL, *tree2 = NULL;
> struct got_diff_blob_output_unidiff_arg arg;
> FILE *f1 = NULL, *f2 = NULL;
> + int fd1 = -1, fd2 = -1;
>
> if (tree_id1) {
> err = got_object_open_as_tree(&tree1, repo, tree_id1);
> @@ -3619,6 +3620,12 @@ diff_trees(struct got_object_id *tree_id1, struct got_
> err = got_error_from_errno("got_opentemp");
> goto done;
> }
> +
> + fd1 = got_opentempfd();
> + if (fd1 == -1) {
> + err = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> }
>
> err = got_object_open_as_tree(&tree2, repo, tree_id2);
> @@ -3630,7 +3637,11 @@ diff_trees(struct got_object_id *tree_id1, struct got_
> err = got_error_from_errno("got_opentemp");
> goto done;
> }
> -
> + fd2 = got_opentempfd();
> + if (fd2 == -1) {
> + err = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> arg.diff_context = diff_context;
> arg.ignore_whitespace = ignore_whitespace;
> arg.force_text_diff = force_text_diff;
> @@ -3639,7 +3650,7 @@ diff_trees(struct got_object_id *tree_id1, struct got_
> arg.nlines = 0;
> while (path[0] == '/')
> path++;
> - err = got_diff_tree(tree1, tree2, f1, f2, path, path, repo,
> + err = got_diff_tree(tree1, tree2, f1, f2, fd1, fd2, path, path, repo,
> got_diff_blob_output_unidiff, &arg, 1);
> done:
> if (tree1)
> @@ -3650,6 +3661,10 @@ done:
> err = got_error_from_errno("fclose");
> if (f2 && fclose(f2) == EOF && err == NULL)
> err = got_error_from_errno("fclose");
> + if (fd1 != -1 && close(fd1) == -1 && err == NULL)
> + err = got_error_from_errno("close");
> + if (fd2 != -1 && close(fd2) == -1 && err == NULL)
> + err = got_error_from_errno("close");
> return err;
> }
>
> @@ -3691,7 +3706,7 @@ get_changed_paths(struct got_pathlist_head *paths,
> if (err)
> goto done;
>
> - err = got_diff_tree(tree1, tree2, NULL, NULL, "", "", repo,
> + err = got_diff_tree(tree1, tree2, NULL, NULL, -1, -1, "", "", repo,
> got_diff_tree_collect_changed_paths, paths, 0);
> done:
> if (tree1)
> @@ -4634,7 +4649,7 @@ print_diff(void *arg, unsigned char status, unsigned c
> struct print_diff_arg *a = arg;
> const struct got_error *err = NULL;
> struct got_blob_object *blob1 = NULL;
> - int fd = -1, fd1 = -1;
> + int fd = -1, fd1 = -1, fd2 = -1;
> FILE *f1 = NULL, *f2 = NULL;
> char *abspath = NULL, *label1 = NULL;
> struct stat sb;
> @@ -4693,7 +4708,17 @@ print_diff(void *arg, unsigned char status, unsigned c
> err = got_error_from_errno("got_opentemp");
> goto done;
> }
> - err = got_diff_objects_as_blobs(NULL, NULL, f1, f2,
> + fd1 = got_opentempfd();
> + if (fd1 == -1) {
> + err = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> + fd2 = got_opentempfd();
> + if (fd2 == -1) {
> + err = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> + err = got_diff_objects_as_blobs(NULL, NULL, f1, f2, fd1, fd2,
> blob_id, staged_blob_id, label1, label2, a->diff_context,
> a->ignore_whitespace, a->force_text_diff, a->repo, stdout);
> goto done;
> @@ -4794,6 +4819,8 @@ print_diff(void *arg, unsigned char status, unsigned c
> done:
> if (fd1 != -1 && close(fd1) == -1 && err == NULL)
> err = got_error_from_errno("close");
> + if (fd2 != -1 && close(fd2) == -1 && err == NULL)
> + err = got_error_from_errno("close");
> if (blob1)
> got_object_blob_close(blob1);
> if (f1 && fclose(f1) == EOF && err == NULL)
> @@ -4825,6 +4852,7 @@ cmd_diff(int argc, char *argv[])
> struct got_pathlist_head paths;
> struct got_pathlist_entry *pe;
> FILE *f1 = NULL, *f2 = NULL;
> + int fd1 = -1, fd2 = -1;
> int *pack_fds = NULL;
>
> TAILQ_INIT(&refs);
> @@ -5134,22 +5162,34 @@ cmd_diff(int argc, char *argv[])
> goto done;
> }
>
> + fd1 = got_opentempfd();
> + if (fd1 == -1) {
> + error = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> +
> + fd2 = got_opentempfd();
> + if (fd2 == -1) {
> + error = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> +
> switch (type1 == GOT_OBJ_TYPE_ANY ? type2 : type1) {
> case GOT_OBJ_TYPE_BLOB:
> error = got_diff_objects_as_blobs(NULL, NULL, f1, f2,
> - ids[0], ids[1], NULL, NULL, diff_context,
> + fd1, fd2, ids[0], ids[1], NULL, NULL, diff_context,
> ignore_whitespace, force_text_diff, repo, stdout);
> break;
> case GOT_OBJ_TYPE_TREE:
> - error = got_diff_objects_as_trees(NULL, NULL, f1, f2,
> + error = got_diff_objects_as_trees(NULL, NULL, f1, f2, fd1, fd2,
> ids[0], ids[1], &paths, "", "", diff_context,
> ignore_whitespace, force_text_diff, repo, stdout);
> break;
> case GOT_OBJ_TYPE_COMMIT:
> printf("diff %s %s\n", labels[0], labels[1]);
> error = got_diff_objects_as_commits(NULL, NULL, f1, f2,
> - ids[0], ids[1], &paths, diff_context, ignore_whitespace,
> - force_text_diff, repo, stdout);
> + fd1, fd2, ids[0], ids[1], &paths, diff_context,
> + ignore_whitespace, force_text_diff, repo, stdout);
> break;
> default:
> error = got_error(GOT_ERR_OBJ_TYPE);
> @@ -5180,6 +5220,10 @@ done:
> error = got_error_from_errno("fclose");
> if (f2 && fclose(f2) == EOF && error == NULL)
> error = got_error_from_errno("fclose");
> + if (fd1 != -1 && close(fd1) == -1 && error == NULL)
> + error = got_error_from_errno("close");
> + if (fd2 != -1 && close(fd2) == -1 && error == NULL)
> + error = got_error_from_errno("close");
> return error;
> }
>
> @@ -9386,7 +9430,7 @@ check_path_prefix(struct got_object_id *parent_id,
> cpp_arg.path_prefix++;
> cpp_arg.len = strlen(cpp_arg.path_prefix);
> cpp_arg.errcode = errcode;
> - err = got_diff_tree(tree1, tree2, NULL, NULL, "", "", repo,
> + err = got_diff_tree(tree1, tree2, NULL, NULL, -1, -1, "", "", repo,
> check_path_prefix_in_diff, &cpp_arg, 0);
> done:
> if (tree1)
> blob - ffc24fe2d445b82dd5d5ff2616a3ce538f584cb8
> blob + 1bfe144b854b965c11288ba9d3508dd0576c7703
> --- gotweb/gotweb.c
> +++ gotweb/gotweb.c
> @@ -2855,6 +2855,7 @@ gw_output_diff(struct gw_trans *gw_trans, struct gw_he
> {
> const struct got_error *error;
> FILE *f = NULL, *f1 = NULL, *f2 = NULL;
> + int fd1 = -1, fd2 = -1;
> struct got_object_id *id1 = NULL, *id2 = NULL;
> char *label1 = NULL, *label2 = NULL, *line = NULL;
> int obj_type;
> @@ -2878,6 +2879,18 @@ gw_output_diff(struct gw_trans *gw_trans, struct gw_he
> goto done;
> }
>
> + fd1 = got_opentempfd();
> + if (fd1 == -1) {
> + error = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> +
> + fd2 = got_opentempfd();
> + if (fd2 == -1) {
> + error = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> +
> if (header->parent_id != NULL &&
> strncmp(header->parent_id, "/dev/null", 9) != 0) {
> error = got_repo_match_object_id(&id1, &label1,
> @@ -2899,15 +2912,17 @@ gw_output_diff(struct gw_trans *gw_trans, struct gw_he
> switch (obj_type) {
> case GOT_OBJ_TYPE_BLOB:
> error = got_diff_objects_as_blobs(NULL, NULL, f1, f2,
> - id1, id2, NULL, NULL, 3, 0, 0, gw_trans->repo, f);
> + fd1, fd2, id1, id2, NULL, NULL, 3, 0, 0,
> + gw_trans->repo, f);
> break;
> case GOT_OBJ_TYPE_TREE:
> error = got_diff_objects_as_trees(NULL, NULL, f1, f2,
> - id1, id2, NULL, "", "", 3, 0, 0, gw_trans->repo, f);
> + fd1, fd2, id1, id2, NULL, "", "", 3, 0, 0,
> + gw_trans->repo, f);
> break;
> case GOT_OBJ_TYPE_COMMIT:
> error = got_diff_objects_as_commits(NULL, NULL, f1, f2,
> - id1, id2, NULL, 3, 0, 0, gw_trans->repo, f);
> + fd1, fd2, id1, id2, NULL, 3, 0, 0, gw_trans->repo, f);
> break;
> default:
> error = got_error(GOT_ERR_OBJ_TYPE);
> @@ -2941,6 +2956,10 @@ done:
> error = got_error_from_errno("fclose");
> if (f2 && fclose(f2) == EOF && error == NULL)
> error = got_error_from_errno("fclose");
> + if (fd1 != -1 && close(fd1) == -1 && error == NULL)
> + error = got_error_from_errno("close");
> + if (fd2 != -1 && close(fd2) == -1 && error == NULL)
> + error = got_error_from_errno("close");
> free(line);
> free(label1);
> free(label2);
> blob - 4f537113e196997bfae7b568a9d90ab8fe9e77b0
> blob + c587513f34441a66ffd34aa5788327dc50f06c7d
> --- include/got_diff.h
> +++ include/got_diff.h
> @@ -102,14 +102,16 @@ const struct got_error *got_diff_blob_output_unidiff(v
> * Diffing of blob content can be suppressed by passing zero for the
> * 'diff_content' parameter. The callback will then only receive blob
> * object IDs and diff labels, but NULL pointers instead of blob objects.
> - * If 'diff_content' is set, two open temporary files must be provided
> - * for internal use; these files can be obtained from got_opentemp()
> + * If 'diff_content' is set, two open temporary FILEs and two open
> + * temporary file descriptors must be provided for internal use; these
> + * files can be obtained from got_opentemp() and got_opentempfd(),
> * and must be closed by the caller. Otherwise the files can be NULL.
> * The set of arguments relating to either tree may be NULL to indicate
> * that no content is present on its respective side of the diff.
> */
> const struct got_error *got_diff_tree(struct got_tree_object *,
> - struct got_tree_object *, FILE *, FILE *, const char *, const char *,
> + struct got_tree_object *, FILE *, FILE *, int, int,
> + const char *, const char *,
> struct got_repository *, got_diff_blob_cb cb, void *cb_arg, int);
>
> /*
> @@ -137,9 +139,10 @@ const struct got_error *got_diff_tree_collect_changed_
> * Diff two objects, assuming both objects are blobs. Two const char * diff
> * header labels may be provided which will be used to identify each blob in
> * the diff output. If a label is NULL, use the blob's SHA1 checksum instead.
> - * Two open temporary files must be provided for internal use; these files
> - * can be obtained from got_opentemp() and must be closed by the caller.
> - * The set of arguments relating to either blob may be NULL to indicate
> + * Two open temporary files and two temporary file descriptors must be
> + * provided for internal use; these files can be obtained from
> + * got_opentemp() and got_opentempfd(), and must be closed by the caller.
> + * The set of arguments relating to either blob may be NULL/-1 to indicate
> * that no content is present on its respective side of the diff.
> * The number of context lines to show in the diff must be specified as well.
> * Write unified diff text to the provided output FILE.
> @@ -147,7 +150,7 @@ const struct got_error *got_diff_tree_collect_changed_
> * array of line offsets for, and the number of lines in, the unidiff text.
> */
> const struct got_error *got_diff_objects_as_blobs(off_t **, size_t *,
> - FILE *, FILE *, struct got_object_id *, struct got_object_id *,
> + FILE *, FILE *, int, int, struct got_object_id *, struct got_object_id *,
> const char *, const char *, int, int, int,
> struct got_repository *, FILE *);
>
> @@ -156,8 +159,10 @@ const struct got_error *got_diff_objects_as_blobs(off_
> * header labels may be provided which will be used to identify each blob in
> * the trees. If a label is NULL, use the blob's SHA1 checksum instead.
> * The number of context lines to show in diffs must be specified.
> - * Two open temporary files must be provided for internal use; these files
> - * can be obtained from got_opentemp() and must be closed by the caller.
> + * Two open temporary files and two temporary file descriptors must be
> + * provided for internal use; these files can be obtained from
> + * got_opentemp() and got_opentempfd(), and must be closed by the caller.
> + * If 'diff_content' is not set, the files may be NULL / -1.
> * The set of arguments relating to either tree may be NULL to indicate
> * that no content is present on its respective side of the diff.
> * Write unified diff text to the provided output FILE.
> @@ -165,15 +170,16 @@ const struct got_error *got_diff_objects_as_blobs(off_
> * array of line offsets for, and the number of lines in, the unidiff text.
> */
> const struct got_error *got_diff_objects_as_trees(off_t **, size_t *,
> - FILE *, FILE *, struct got_object_id *, struct got_object_id *,
> + FILE *, FILE *, int, int, struct got_object_id *, struct got_object_id *,
> struct got_pathlist_head *, const char *, const char *, int, int, int,
> struct got_repository *, FILE *);
>
> /*
> * Diff two objects, assuming both objects are commits.
> * The number of context lines to show in diffs must be specified.
> - * Two open temporary files must be provided for internal use; these files
> - * can be obtained from got_opentemp() and must be closed by the caller.
> + * Two open temporary files and two temporary file descriptors must be
> + * provided for internal use; these files can be obtained from
> + * got_opentemp() and got_opentempfd(), and must be closed by the caller.
> * The set of arguments relating to either commit may be NULL to indicate
> * that no content is present on its respective side of the diff.
> * Write unified diff text to the provided output FILE.
> @@ -181,7 +187,7 @@ const struct got_error *got_diff_objects_as_trees(off_
> * array of line offsets for, and the number of lines in, the unidiff text.
> */
> const struct got_error *got_diff_objects_as_commits(off_t **, size_t *,
> - FILE *, FILE *, struct got_object_id *, struct got_object_id *,
> + FILE *, FILE *, int, int, struct got_object_id *, struct got_object_id *,
> struct got_pathlist_head *, int, int, int, struct got_repository *, FILE *);
>
> #define GOT_DIFF_MAX_CONTEXT 64
> blob - 7bc3ad07ee796048b5285aabebb4e367fe9de9d5
> blob + 13a1b0bf323c1c003dbb7b2473b2d1a20c202a43
> --- lib/diff.c
> +++ lib/diff.c
> @@ -273,25 +273,18 @@ got_diff_blob_file(struct got_blob_object *blob1, FILE
> }
>
> static const struct got_error *
> -diff_added_blob(struct got_object_id *id, FILE *f, const char *label,
> - mode_t mode, struct got_repository *repo,
> +diff_added_blob(struct got_object_id *id, FILE *f, int fd,
> + const char *label, mode_t mode, struct got_repository *repo,
> got_diff_blob_cb cb, void *cb_arg)
> {
> const struct got_error *err;
> struct got_blob_object *blob = NULL;
> struct got_object *obj = NULL;
> - int fd = -1;
>
> err = got_object_open(&obj, repo, id);
> if (err)
> return err;
>
> - fd = got_opentempfd();
> - if (fd == -1) {
> - err = got_error_from_errno("got_opentempfd");
> - goto done;
> - }
> -
> err = got_object_blob_open(&blob, repo, obj, 8192, fd);
> if (err)
> goto done;
> @@ -299,8 +292,6 @@ diff_added_blob(struct got_object_id *id, FILE *f, con
> NULL, label, 0, mode, repo);
> done:
> got_object_close(obj);
> - if (fd != -1 && close(fd) == -1 && err == NULL)
> - err = got_error_from_errno("close");
> if (blob)
> got_object_blob_close(blob);
> return err;
> @@ -308,7 +299,8 @@ done:
>
> static const struct got_error *
> diff_modified_blob(struct got_object_id *id1, struct got_object_id *id2,
> - FILE *f1, FILE *f2, const char *label1, const char *label2,
> + FILE *f1, FILE *f2, int fd1, int fd2,
> + const char *label1, const char *label2,
> mode_t mode1, mode_t mode2, struct got_repository *repo,
> got_diff_blob_cb cb, void *cb_arg)
> {
> @@ -317,23 +309,11 @@ diff_modified_blob(struct got_object_id *id1, struct g
> struct got_object *obj2 = NULL;
> struct got_blob_object *blob1 = NULL;
> struct got_blob_object *blob2 = NULL;
> - int fd1 = -1, fd2 = -1;
>
> err = got_object_open(&obj1, repo, id1);
> if (err)
> return err;
>
> - fd1 = got_opentempfd();
> - if (fd1 == -1) {
> - err = got_error_from_errno("got_opentempfd");
> - goto done;
> - }
> - fd2 = got_opentempfd();
> - if (fd2 == -1) {
> - err = got_error_from_errno("got_opentempfd");
> - goto done;
> - }
> -
> if (obj1->type != GOT_OBJ_TYPE_BLOB) {
> err = got_error(GOT_ERR_OBJ_TYPE);
> goto done;
> @@ -362,30 +342,22 @@ done:
> got_object_close(obj1);
> if (obj2)
> got_object_close(obj2);
> - if (fd1 != -1 && close(fd1) == -1 && err == NULL)
> - err = got_error_from_errno("close");
> if (blob1)
> got_object_blob_close(blob1);
> - if (fd2 != -1 && close(fd2) == -1 && err == NULL)
> - err = got_error_from_errno("close");
> if (blob2)
> got_object_blob_close(blob2);
> return err;
> }
>
> static const struct got_error *
> -diff_deleted_blob(struct got_object_id *id, FILE *f, const char *label,
> - mode_t mode, struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg)
> +diff_deleted_blob(struct got_object_id *id, FILE *f, int fd,
> + const char *label, mode_t mode, struct got_repository *repo,
> + got_diff_blob_cb cb, void *cb_arg)
> {
> const struct got_error *err;
> struct got_blob_object *blob = NULL;
> struct got_object *obj = NULL;
> - int fd = -1;
>
> - fd = got_opentempfd();
> - if (fd == -1)
> - return got_error_from_errno("got_opentempfd");
> -
> err = got_object_open(&obj, repo, id);
> if (err)
> return err;
> @@ -397,15 +369,13 @@ diff_deleted_blob(struct got_object_id *id, FILE *f, c
> mode, 0, repo);
> done:
> got_object_close(obj);
> - if (fd != -1 && close(fd) == -1 && err == NULL)
> - err = got_error_from_errno("close");
> if (blob)
> got_object_blob_close(blob);
> return err;
> }
>
> static const struct got_error *
> -diff_added_tree(struct got_object_id *id, FILE *f, const char *label,
> +diff_added_tree(struct got_object_id *id, FILE *f, int fd, const char *label,
> struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg,
> int diff_content)
> {
> @@ -426,7 +396,7 @@ diff_added_tree(struct got_object_id *id, FILE *f, con
> if (err)
> goto done;
>
> - err = got_diff_tree(NULL, tree, NULL, f, NULL, label,
> + err = got_diff_tree(NULL, tree, NULL, f, -1, fd, NULL, label,
> repo, cb, cb_arg, diff_content);
> done:
> if (tree)
> @@ -438,7 +408,8 @@ done:
>
> static const struct got_error *
> diff_modified_tree(struct got_object_id *id1, struct got_object_id *id2,
> - FILE *f1, FILE *f2, const char *label1, const char *label2,
> + FILE *f1, FILE *f2, int fd1, int fd2,
> + const char *label1, const char *label2,
> struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg,
> int diff_content)
> {
> @@ -474,8 +445,8 @@ diff_modified_tree(struct got_object_id *id1, struct g
> if (err)
> goto done;
>
> - err = got_diff_tree(tree1, tree2, f1, f2, label1, label2,
> - repo, cb, cb_arg, diff_content);
> + err = got_diff_tree(tree1, tree2, f1, f2, fd1, fd2,
> + label1, label2, repo, cb, cb_arg, diff_content);
>
> done:
> if (tree1)
> @@ -490,9 +461,9 @@ done:
> }
>
> static const struct got_error *
> -diff_deleted_tree(struct got_object_id *id, FILE *f, const char *label,
> - struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg,
> - int diff_content)
> +diff_deleted_tree(struct got_object_id *id, FILE *f, int fd,
> + const char *label, struct got_repository *repo,
> + got_diff_blob_cb cb, void *cb_arg, int diff_content)
> {
> const struct got_error *err;
> struct got_object *treeobj = NULL;
> @@ -511,7 +482,7 @@ diff_deleted_tree(struct got_object_id *id, FILE *f, c
> if (err)
> goto done;
>
> - err = got_diff_tree(tree, NULL, f, NULL, label, NULL,
> + err = got_diff_tree(tree, NULL, f, NULL, fd, -1, label, NULL,
> repo, cb, cb_arg, diff_content);
> done:
> if (tree)
> @@ -532,7 +503,8 @@ diff_kind_mismatch(struct got_object_id *id1, struct g
>
> static const struct got_error *
> diff_entry_old_new(struct got_tree_entry *te1, struct got_tree_entry *te2,
> - FILE *f1, FILE *f2, const char *label1, const char *label2,
> + FILE *f1, FILE *f2, int fd1, int fd2,
> + const char *label1, const char *label2,
> struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg,
> int diff_content)
> {
> @@ -544,12 +516,12 @@ diff_entry_old_new(struct got_tree_entry *te1, struct
>
> if (te2 == NULL) {
> if (S_ISDIR(te1->mode))
> - err = diff_deleted_tree(&te1->id, f1, label1, repo,
> - cb, cb_arg, diff_content);
> + err = diff_deleted_tree(&te1->id, f1, fd1, label1,
> + repo, cb, cb_arg, diff_content);
> else {
> if (diff_content)
> - err = diff_deleted_blob(&te1->id, f1, label1,
> - te1->mode, repo, cb, cb_arg);
> + err = diff_deleted_blob(&te1->id, f1, fd1,
> + label1, te1->mode, repo, cb, cb_arg);
> else
> err = cb(cb_arg, NULL, NULL, NULL, NULL,
> &te1->id, NULL, label1, NULL,
> @@ -563,7 +535,8 @@ diff_entry_old_new(struct got_tree_entry *te1, struct
> if (S_ISDIR(te1->mode) && S_ISDIR(te2->mode)) {
> if (!id_match)
> return diff_modified_tree(&te1->id, &te2->id, f1, f2,
> - label1, label2, repo, cb, cb_arg, diff_content);
> + fd1, fd2, label1, label2, repo, cb, cb_arg,
> + diff_content);
> } else if ((S_ISREG(te1->mode) || S_ISLNK(te1->mode)) &&
> (S_ISREG(te2->mode) || S_ISLNK(te2->mode))) {
> if (!id_match ||
> @@ -571,8 +544,8 @@ diff_entry_old_new(struct got_tree_entry *te1, struct
> (te2->mode & (S_IFLNK | S_IXUSR))) {
> if (diff_content)
> return diff_modified_blob(&te1->id, &te2->id,
> - f1, f2, label1, label2, te1->mode,
> - te2->mode, repo, cb, cb_arg);
> + f1, f2, fd1, fd2, label1, label2,
> + te1->mode, te2->mode, repo, cb, cb_arg);
> else
> return cb(cb_arg, NULL, NULL, NULL, NULL,
> &te1->id, &te2->id, label1, label2,
> @@ -589,7 +562,7 @@ diff_entry_old_new(struct got_tree_entry *te1, struct
>
> static const struct got_error *
> diff_entry_new_old(struct got_tree_entry *te2,
> - struct got_tree_entry *te1, FILE *f2, const char *label2,
> + struct got_tree_entry *te1, FILE *f2, int fd2, const char *label2,
> struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg,
> int diff_content)
> {
> @@ -600,12 +573,12 @@ diff_entry_new_old(struct got_tree_entry *te2,
> return NULL;
>
> if (S_ISDIR(te2->mode))
> - return diff_added_tree(&te2->id, f2, label2,
> + return diff_added_tree(&te2->id, f2, fd2, label2,
> repo, cb, cb_arg, diff_content);
>
> if (diff_content)
> - return diff_added_blob(&te2->id, f2, label2, te2->mode,
> - repo, cb, cb_arg);
> + return diff_added_blob(&te2->id, f2, fd2,
> + label2, te2->mode, repo, cb, cb_arg);
>
> return cb(cb_arg, NULL, NULL, NULL, NULL, NULL, &te2->id,
> NULL, label2, 0, te2->mode, repo);
> @@ -656,7 +629,8 @@ done:
>
> const struct got_error *
> got_diff_tree(struct got_tree_object *tree1, struct got_tree_object *tree2,
> - FILE *f1, FILE *f2, const char *label1, const char *label2,
> + FILE *f1, FILE *f2, int fd1, int fd2,
> + const char *label1, const char *label2,
> struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg,
> int diff_content)
> {
> @@ -693,8 +667,8 @@ got_diff_tree(struct got_tree_object *tree1, struct go
> return
> got_error_from_errno("asprintf");
> }
> - err = diff_entry_old_new(te1, te, f1, f2, l1, l2,
> - repo, cb, cb_arg, diff_content);
> + err = diff_entry_old_new(te1, te, f1, f2, fd1, fd2,
> + l1, l2, repo, cb, cb_arg, diff_content);
> if (err)
> break;
> }
> @@ -716,7 +690,7 @@ got_diff_tree(struct got_tree_object *tree1, struct go
> return
> got_error_from_errno("asprintf");
> }
> - err = diff_entry_new_old(te2, te, f2, l2, repo,
> + err = diff_entry_new_old(te2, te, f2, fd2, l2, repo,
> cb, cb_arg, diff_content);
> if (err)
> break;
> @@ -749,27 +723,18 @@ got_diff_tree(struct got_tree_object *tree1, struct go
>
> const struct got_error *
> got_diff_objects_as_blobs(off_t **line_offsets, size_t *nlines,
> - FILE *f1, FILE *f2, struct got_object_id *id1, struct got_object_id *id2,
> + FILE *f1, FILE *f2, int fd1, int fd2,
> + struct got_object_id *id1, struct got_object_id *id2,
> const char *label1, const char *label2, int diff_context,
> int ignore_whitespace, int force_text_diff,
> struct got_repository *repo, FILE *outfile)
> {
> const struct got_error *err;
> struct got_blob_object *blob1 = NULL, *blob2 = NULL;
> - int fd1 = -1, fd2 = -1;
>
> if (id1 == NULL && id2 == NULL)
> return got_error(GOT_ERR_NO_OBJ);
>
> - fd1 = got_opentempfd();
> - if (fd1 == -1)
> - return got_error_from_errno("got_opentempfd");
> - fd2 = got_opentempfd();
> - if (fd2 == -1) {
> - err = got_error_from_errno("got_opentempfd");
> - goto done;
> - }
> -
> if (id1) {
> err = got_object_open_as_blob(&blob1, repo, id1, 8192, fd1);
> if (err)
> @@ -784,12 +749,8 @@ got_diff_objects_as_blobs(off_t **line_offsets, size_t
> label1, label2, diff_context, ignore_whitespace, force_text_diff,
> outfile);
> done:
> - if (fd1 != -1 && close(fd1) == -1 && err == NULL)
> - err = got_error_from_errno("close");
> if (blob1)
> got_object_blob_close(blob1);
> - if (fd2 != -1 && close(fd2) == -1 && err == NULL)
> - err = got_error_from_errno("close");
> if (blob2)
> got_object_blob_close(blob2);
> return err;
> @@ -797,7 +758,7 @@ done:
>
> static const struct got_error *
> diff_paths(struct got_tree_object *tree1, struct got_tree_object *tree2,
> - FILE *f1, FILE *f2, struct got_pathlist_head *paths,
> + FILE *f1, FILE *f2, int fd1, int fd2, struct got_pathlist_head *paths,
> struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg)
> {
> const struct got_error *err = NULL;
> @@ -805,16 +766,7 @@ diff_paths(struct got_tree_object *tree1, struct got_t
> struct got_object_id *id1 = NULL, *id2 = NULL;
> struct got_tree_object *subtree1 = NULL, *subtree2 = NULL;
> struct got_blob_object *blob1 = NULL, *blob2 = NULL;
> - int fd1 = -1, fd2 = -1;
>
> - fd1 = got_opentempfd();
> - if (fd1 == -1)
> - return got_error_from_errno("got_opentempfd");
> - fd2 = got_opentempfd();
> - if (fd2 == -1) {
> - err = got_error_from_errno("got_opentempfd");
> - goto done;
> - }
> TAILQ_FOREACH(pe, paths, entry) {
> int type1 = GOT_OBJ_TYPE_ANY, type2 = GOT_OBJ_TYPE_ANY;
> mode_t mode1 = 0, mode2 = 0;
> @@ -907,6 +859,7 @@ diff_paths(struct got_tree_object *tree1, struct got_t
> goto done;
> }
> err = got_diff_tree(subtree1, subtree2, f1, f2,
> + fd1, fd2,
> id1 ? pe->path : "/dev/null",
> id2 ? pe->path : "/dev/null",
> repo, cb, cb_arg, 1);
> @@ -916,10 +869,6 @@ diff_paths(struct got_tree_object *tree1, struct got_t
> err = got_error(GOT_ERR_OBJ_TYPE);
> goto done;
> }
> - if (ftruncate(fd1, 0L) == -1)
> - return got_error_from_errno("ftruncate");
> - if (ftruncate(fd2, 0L) == -1)
> - return got_error_from_errno("ftruncate");
> }
> done:
> free(id1);
> @@ -928,12 +877,8 @@ done:
> got_object_tree_close(subtree1);
> if (subtree2)
> got_object_tree_close(subtree2);
> - if (fd1 != -1 && close(fd1) == -1 && err == NULL)
> - err = got_error_from_errno("close");
> if (blob1)
> got_object_blob_close(blob1);
> - if (fd2 != -1 && close(fd2) == -1 && err == NULL)
> - err = got_error_from_errno("close");
> if (blob2)
> got_object_blob_close(blob2);
> return err;
> @@ -967,7 +912,8 @@ show_object_id(off_t **line_offsets, size_t *nlines, c
>
> static const struct got_error *
> diff_objects_as_trees(off_t **line_offsets, size_t *nlines,
> - FILE *f1, FILE *f2, struct got_object_id *id1, struct got_object_id *id2,
> + FILE *f1, FILE *f2, int fd1, int fd2,
> + struct got_object_id *id1, struct got_object_id *id2,
> struct got_pathlist_head *paths, const char *label1, const char *label2,
> int diff_context, int ignore_whitespace, int force_text_diff,
> struct got_repository *repo, FILE *outfile)
> @@ -1003,10 +949,11 @@ diff_objects_as_trees(off_t **line_offsets, size_t *nl
> arg.nlines = 0;
> }
> if (paths == NULL || TAILQ_EMPTY(paths)) {
> - err = got_diff_tree(tree1, tree2, f1, f2, label1, label2,
> - repo, got_diff_blob_output_unidiff, &arg, 1);
> + err = got_diff_tree(tree1, tree2, f1, f2, fd1, fd2,
> + label1, label2, repo,
> + got_diff_blob_output_unidiff, &arg, 1);
> } else {
> - err = diff_paths(tree1, tree2, f1, f2, paths, repo,
> + err = diff_paths(tree1, tree2, f1, f2, fd1, fd2, paths, repo,
> got_diff_blob_output_unidiff, &arg);
> }
> if (want_lineoffsets) {
> @@ -1023,7 +970,8 @@ done:
>
> const struct got_error *
> got_diff_objects_as_trees(off_t **line_offsets, size_t *nlines,
> - FILE *f1, FILE *f2, struct got_object_id *id1, struct got_object_id *id2,
> + FILE *f1, FILE *f2, int fd1, int fd2,
> + struct got_object_id *id1, struct got_object_id *id2,
> struct got_pathlist_head *paths, const char *label1, const char *label2,
> int diff_context, int ignore_whitespace, int force_text_diff,
> struct got_repository *repo, FILE *outfile)
> @@ -1068,8 +1016,8 @@ got_diff_objects_as_trees(off_t **line_offsets, size_t
> goto done;
> }
>
> - err = diff_objects_as_trees(line_offsets, nlines, f1, f2, id1, id2,
> - paths, label1, label2, diff_context, ignore_whitespace,
> + err = diff_objects_as_trees(line_offsets, nlines, f1, f2, fd1, fd2,
> + id1, id2, paths, label1, label2, diff_context, ignore_whitespace,
> force_text_diff, repo, outfile);
> done:
> free(idstr);
> @@ -1078,7 +1026,8 @@ done:
>
> const struct got_error *
> got_diff_objects_as_commits(off_t **line_offsets, size_t *nlines,
> - FILE *f1, FILE *f2, struct got_object_id *id1, struct got_object_id *id2,
> + FILE *f1, FILE *f2, int fd1, int fd2,
> + struct got_object_id *id1, struct got_object_id *id2,
> struct got_pathlist_head *paths,
> int diff_context, int ignore_whitespace, int force_text_diff,
> struct got_repository *repo, FILE *outfile)
> @@ -1122,7 +1071,7 @@ got_diff_objects_as_commits(off_t **line_offsets, size
> if (err)
> goto done;
>
> - err = diff_objects_as_trees(line_offsets, nlines, f1, f2,
> + err = diff_objects_as_trees(line_offsets, nlines, f1, f2, fd1, fd2,
> commit1 ? got_object_commit_get_tree_id(commit1) : NULL,
> got_object_commit_get_tree_id(commit2), paths, "", "",
> diff_context, ignore_whitespace, force_text_diff, repo, outfile);
> blob - 8decec3748ea2448a4255cba6eccf77ced9fab2a
> blob + 3ffd653ef429fab490d06ba6a953185254e7c117
> --- lib/error.c
> +++ lib/error.c
> @@ -236,6 +236,7 @@ got_error(int code)
> {
> size_t i;
>
> + if (code == GOT_ERR_PRIVSEP_LEN) abort();
> for (i = 0; i < nitems(got_errors); i++) {
> if (code == got_errors[i].code)
> return &got_errors[i];
> blob - d2ba8bff2999366cb470b0e5b15c07f8d2a96442
> blob + 12ae8e569a5cb16540cc4d70ad34d56c0964258a
> --- lib/object.c
> +++ lib/object.c
> @@ -1368,6 +1368,15 @@ open_blob(struct got_blob_object **blob, struct got_re
> goto done;
> }
>
> + if (ftruncate(outfd, 0L) == -1) {
> + err = got_error_from_errno("ftruncate");
> + goto done;
> + }
> + if (lseek(outfd, SEEK_SET, 0) == -1) {
> + err = got_error_from_errno("lseek");
> + goto done;
> + }
> +
> err = got_repo_search_packidx(&packidx, &idx, repo, id);
> if (err == NULL) {
> struct got_pack *pack = NULL;
> blob - bde5fed8d73d8b12076780fe47b0782a4fdb69db
> blob + 18f13a154b509692bfadad8d3d5873d52e5bd66b
> --- lib/worktree.c
> +++ lib/worktree.c
> @@ -3136,6 +3136,7 @@ merge_files(struct got_worktree *worktree, struct got_
> struct merge_file_cb_arg arg;
> char *label_orig = NULL;
> FILE *f1 = NULL, *f2 = NULL;
> + int fd1 = -1, fd2 = -1;
>
> if (commit_id1) {
> err = got_object_open_as_commit(&commit1, repo, commit_id1);
> @@ -3191,10 +3192,22 @@ merge_files(struct got_worktree *worktree, struct got_
> goto done;
> }
>
> + fd1 = got_opentempfd();
> + if (fd1 == -1) {
> + err = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> +
> + fd2 = got_opentempfd();
> + if (fd2 == -1) {
> + err = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> +
> cmc_arg.worktree = worktree;
> cmc_arg.fileindex = fileindex;
> cmc_arg.repo = repo;
> - err = got_diff_tree(tree1, tree2, f1, f2, "", "", repo,
> + err = got_diff_tree(tree1, tree2, f1, f2, fd1, fd2, "", "", repo,
> check_merge_conflicts, &cmc_arg, 0);
> if (err)
> goto done;
> @@ -3208,7 +3221,7 @@ merge_files(struct got_worktree *worktree, struct got_
> arg.label_orig = label_orig;
> arg.commit_id2 = commit_id2;
> arg.allow_bad_symlinks = 1; /* preserve bad symlinks across merges */
> - err = got_diff_tree(tree1, tree2, f1, f2, "", "", repo,
> + err = got_diff_tree(tree1, tree2, f1, f2, fd1, fd2, "", "", repo,
> merge_file_cb, &arg, 1);
> sync_err = sync_fileindex(fileindex, fileindex_path);
> if (sync_err && err == NULL)
> @@ -3226,6 +3239,10 @@ done:
> err = got_error_from_errno("fclose");
> if (f2 && fclose(f2) == EOF && err == NULL)
> err = got_error_from_errno("fclose");
> + if (fd1 != -1 && close(fd1) == -1 && err == NULL)
> + err = got_error_from_errno("close");
> + if (fd2 != -1 && close(fd2) == -1 && err == NULL)
> + err = got_error_from_errno("close");
> free(label_orig);
> return err;
> }
> blob - 02357b5d2aa6509aa4c4c341f9bc46ba93cf213d
> blob + 609e8f12ee4e83af2f66a84f1b0ad9b823991113
> --- tog/tog.c
> +++ tog/tog.c
> @@ -309,6 +309,7 @@ struct tog_diff_view_state {
> struct got_object_id *id1, *id2;
> const char *label1, *label2;
> FILE *f, *f1, *f2;
> + int fd1, fd2;
> int first_displayed_line;
> int last_displayed_line;
> int eof;
> @@ -3504,7 +3505,7 @@ get_changed_paths(struct got_pathlist_head *paths,
> if (err)
> goto done;
>
> - err = got_diff_tree(tree1, tree2, NULL, NULL, "", "", repo,
> + err = got_diff_tree(tree1, tree2, NULL, NULL, -1, -1, "", "", repo,
> got_diff_tree_collect_changed_paths, paths, 0);
> done:
> if (tree1)
> @@ -3724,14 +3725,15 @@ create_diff(struct tog_diff_view_state *s)
> switch (obj_type) {
> case GOT_OBJ_TYPE_BLOB:
> err = got_diff_objects_as_blobs(&s->line_offsets, &s->nlines,
> - s->f1, s->f2, s->id1, s->id2, s->label1, s->label2,
> - s->diff_context, s->ignore_whitespace, s->force_text_diff,
> - s->repo, s->f);
> + s->f1, s->f2, s->fd1, s->fd2, s->id1, s->id2,
> + s->label1, s->label2, s->diff_context,
> + s->ignore_whitespace, s->force_text_diff, s->repo, s->f);
> break;
> case GOT_OBJ_TYPE_TREE:
> err = got_diff_objects_as_trees(&s->line_offsets, &s->nlines,
> - s->f1, s->f2, s->id1, s->id2, NULL, "", "", s->diff_context,
> - s->ignore_whitespace, s->force_text_diff, s->repo, s->f);
> + s->f1, s->f2, s->fd1, s->fd2, s->id1, s->id2, NULL, "", "",
> + s->diff_context, s->ignore_whitespace, s->force_text_diff,
> + s->repo, s->f);
> break;
> case GOT_OBJ_TYPE_COMMIT: {
> const struct got_object_id_queue *parent_ids;
> @@ -3765,8 +3767,9 @@ create_diff(struct tog_diff_view_state *s)
> got_object_commit_close(commit2);
>
> err = got_diff_objects_as_commits(&s->line_offsets, &s->nlines,
> - s->f1, s->f2, s->id1, s->id2, NULL, s->diff_context,
> - s->ignore_whitespace, s->force_text_diff, s->repo, s->f);
> + s->f1, s->f2, s->fd1, s->fd2, s->id1, s->id2, NULL,
> + s->diff_context, s->ignore_whitespace, s->force_text_diff,
> + s->repo, s->f);
> break;
> }
> default:
> @@ -3884,12 +3887,18 @@ close_diff_view(struct tog_view *view)
> if (s->f && fclose(s->f) == EOF)
> err = got_error_from_errno("fclose");
> s->f = NULL;
> - if (s->f1 && fclose(s->f1) == EOF)
> + if (s->f1 && fclose(s->f1) == EOF && err == NULL)
> err = got_error_from_errno("fclose");
> s->f1 = NULL;
> - if (s->f2 && fclose(s->f2) == EOF)
> + if (s->f2 && fclose(s->f2) == EOF && err == NULL)
> err = got_error_from_errno("fclose");
> s->f2 = NULL;
> + if (s->fd1 != -1 && close(s->fd1) == -1 && err == NULL)
> + err = got_error_from_errno("close");
> + s->fd1 = -1;
> + if (s->fd2 != -1 && close(s->fd2) == -1 && err == NULL)
> + err = got_error_from_errno("close");
> + s->fd2 = -1;
> free_colors(&s->colors);
> free(s->line_offsets);
> s->line_offsets = NULL;
> @@ -3907,6 +3916,8 @@ open_diff_view(struct tog_view *view, struct got_objec
> struct tog_diff_view_state *s = &view->state.diff;
>
> memset(s, 0, sizeof(*s));
> + s->fd1 = -1;
> + s->fd2 = -1;
>
> if (id1 != NULL && id2 != NULL) {
> int type1, type2;
> @@ -3954,6 +3965,18 @@ open_diff_view(struct tog_view *view, struct got_objec
> goto done;
> }
>
> + s->fd1 = got_opentempfd();
> + if (s->fd1 == -1) {
> + err = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> +
> + s->fd2 = got_opentempfd();
> + if (s->fd2 == -1) {
> + err = got_error_from_errno("got_opentempfd");
> + goto done;
> + }
> +
> s->first_displayed_line = 1;
> s->last_displayed_line = view->nlines;
> s->diff_context = diff_context;
>
--
Tracey Emery
move got_opentempfd() out of lib/diff.c again