move got_opentempfd() out of lib/diff.c again

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? 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;

2022-06-28 20:08 Stefan Sperling:
move got_opentempfd() out of lib/diff.c again
- 2022-06-28 20:31 Tracey Emery:
  move got_opentempfd() out of lib/diff.c again