From: Stefan Sperling Subject: move opentemp out of diffreg.c To: gameoftrees@openbsd.org Date: Thu, 30 Jun 2022 12:21:17 +0200 A few more got_opentemp() calls which gotwebd will trip over exist in lib/diffreg.c. This patch moves them up to callers. ok? diff 9b058f456d15d60a89334ce3e7f0a7c22e182c55 c235d605cca5f3d7bda019aefd04505ea97a7ac8 commit - 9b058f456d15d60a89334ce3e7f0a7c22e182c55 commit + c235d605cca5f3d7bda019aefd04505ea97a7ac8 blob - 2dceeed3a7a54bee61a491f92527df8295059f95 blob + 277183116cdb51d8f491172df6f141ad77e57c26 --- got/got.c +++ got/got.c @@ -3564,17 +3564,17 @@ diff_blobs(struct got_object_id *blob_id1, struct got_ fd1); if (err) goto done; - f1 = got_opentemp(); - if (f1 == NULL) { - err = got_error_from_errno("got_opentemp"); - goto done; - } } err = got_object_open_as_blob(&blob2, repo, blob_id2, 8192, fd2); if (err) goto done; + f1 = got_opentemp(); + if (f1 == NULL) { + err = got_error_from_errno("got_opentemp"); + goto done; + } f2 = got_opentemp(); if (f2 == NULL) { err = got_error_from_errno("got_opentemp"); @@ -3615,12 +3615,6 @@ diff_trees(struct got_object_id *tree_id1, struct got_ err = got_object_open_as_tree(&tree1, repo, tree_id1); if (err) goto done; - f1 = got_opentemp(); - if (f1 == NULL) { - err = got_error_from_errno("got_opentemp"); - goto done; - } - fd1 = got_opentempfd(); if (fd1 == -1) { err = got_error_from_errno("got_opentempfd"); @@ -3632,6 +3626,12 @@ diff_trees(struct got_object_id *tree_id1, struct got_ if (err) goto done; + f1 = got_opentemp(); + if (f1 == NULL) { + err = got_error_from_errno("got_opentemp"); + goto done; + } + f2 = got_opentemp(); if (f2 == NULL) { err = got_error_from_errno("got_opentemp"); @@ -4592,6 +4592,8 @@ struct print_diff_arg { int diff_staged; int ignore_whitespace; int force_text_diff; + FILE *f1; + FILE *f2; }; /* @@ -4650,10 +4652,11 @@ print_diff(void *arg, unsigned char status, unsigned c const struct got_error *err = NULL; struct got_blob_object *blob1 = NULL; int fd = -1, fd1 = -1, fd2 = -1; - FILE *f1 = NULL, *f2 = NULL; + FILE *f2 = NULL; char *abspath = NULL, *label1 = NULL; struct stat sb; off_t size1 = 0; + int f2_exists = 1; if (a->diff_staged) { if (staged_status != GOT_STATUS_MODIFY && @@ -4672,6 +4675,13 @@ print_diff(void *arg, unsigned char status, unsigned c return NULL; } + err = got_opentemp_truncate(a->f1); + if (err) + return got_error_from_errno("got_opentemp_truncate"); + err = got_opentemp_truncate(a->f2); + if (err) + return got_error_from_errno("got_opentemp_truncate"); + if (!a->header_shown) { printf("diff %s%s\n", a->diff_staged ? "-s " : "", got_worktree_get_root_path(a->worktree)); @@ -4698,16 +4708,6 @@ print_diff(void *arg, unsigned char status, unsigned c default: return got_error(GOT_ERR_FILE_STATUS); } - f1 = got_opentemp(); - if (f1 == NULL) { - err = got_error_from_errno("got_opentemp"); - goto done; - } - f2 = got_opentemp(); - if (f2 == NULL) { - err = got_error_from_errno("got_opentemp"); - goto done; - } fd1 = got_opentempfd(); if (fd1 == -1) { err = got_error_from_errno("got_opentempfd"); @@ -4718,9 +4718,10 @@ print_diff(void *arg, unsigned char status, unsigned c 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); + err = got_diff_objects_as_blobs(NULL, NULL, a->f1, a->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; } @@ -4798,24 +4799,21 @@ print_diff(void *arg, unsigned char status, unsigned c goto done; } fd = -1; - } else + } else { sb.st_size = 0; + f2_exists = 0; + } if (blob1) { - f1 = got_opentemp(); - if (f1 == NULL) { - err = got_error_from_errno("got_opentemp"); - goto done; - } - err = got_object_blob_dump_to_file(&size1, NULL, NULL, f1, - blob1); + err = got_object_blob_dump_to_file(&size1, NULL, NULL, + a->f1, blob1); if (err) goto done; } - err = got_diff_blob_file(blob1, f1, size1, label1, f2, sb.st_size, - path, a->diff_context, a->ignore_whitespace, a->force_text_diff, - stdout); + err = got_diff_blob_file(blob1, a->f1, size1, label1, f2 ? f2 : a->f2, + f2_exists, sb.st_size, path, a->diff_context, a->ignore_whitespace, + a->force_text_diff, stdout); done: if (fd1 != -1 && close(fd1) == -1 && err == NULL) err = got_error_from_errno("close"); @@ -4823,12 +4821,10 @@ done: err = got_error_from_errno("close"); if (blob1) got_object_blob_close(blob1); - if (f1 && fclose(f1) == EOF && err == NULL) - err = got_error_from_errno("fclose"); - if (f2 && fclose(f2) == EOF && err == NULL) - err = got_error_from_errno("fclose"); if (fd != -1 && close(fd) == -1 && err == NULL) err = got_error_from_errno("close"); + if (f2 && fclose(f2) == EOF && err == NULL) + err = got_error_from_errno("fclose"); free(abspath); return err; } @@ -4991,6 +4987,18 @@ cmd_diff(int argc, char *argv[]) } } + f1 = got_opentemp(); + if (f1 == NULL) { + error = got_error_from_errno("got_opentemp"); + goto done; + } + + f2 = got_opentemp(); + if (f2 == NULL) { + error = got_error_from_errno("got_opentemp"); + goto done; + } + if (ncommit_args == 0 && (ids[0] == NULL || ids[1] == NULL)) { struct print_diff_arg arg; char *id_str; @@ -5029,6 +5037,8 @@ cmd_diff(int argc, char *argv[]) arg.diff_staged = diff_staged; arg.ignore_whitespace = ignore_whitespace; arg.force_text_diff = force_text_diff; + arg.f1 = f1; + arg.f2 = f2; error = got_worktree_status(worktree, &paths, repo, 0, print_diff, &arg, check_cancelled, NULL); @@ -5150,18 +5160,6 @@ cmd_diff(int argc, char *argv[]) worktree = NULL; } - f1 = got_opentemp(); - if (f1 == NULL) { - error = got_error_from_errno("got_opentemp"); - goto done; - } - - f2 = got_opentemp(); - if (f2 == NULL) { - error = got_error_from_errno("got_opentemp"); - goto done; - } - fd1 = got_opentempfd(); if (fd1 == -1) { error = got_error_from_errno("got_opentempfd"); blob - c587513f34441a66ffd34aa5788327dc50f06c7d blob + e04cc7fd9e3459de5ae8004badfe6704feaea240 --- include/got_diff.h +++ include/got_diff.h @@ -20,11 +20,9 @@ * for internal use; these files can be obtained from got_opentemp() and * must be closed by the caller. * If one of the blobs being diffed does not exist, all corresponding - * blob object and temporary file arguments should be set to NULL. + * blob object arguments should be set to NULL. * Two const char * diff header labels may be provided which will be used * to identify each blob in the diff output. - * The set of arguments relating to either blob may be NULL to indicate - * that no content is present on its respective side of the diff. * If a label is NULL, use the blob's SHA1 checksum instead. * The number of context lines to show in the diff must be specified as well. * Whitespace differences may optionally be ignored. @@ -38,14 +36,15 @@ const struct got_error *got_diff_blob(off_t **, size_t /* * Compute the differences between a blob and a file and write unified diff * text to the provided output file. The blob object, its content, and its - * size must be provided.The file's size must be provided, as well as a + * size must be provided. The file's size must be provided, as well as a * const char * diff header label which identifies the file. * An optional const char * diff header label for the blob may be provided, too. * The number of context lines to show in the diff must be specified as well. * Whitespace differences may optionally be ignored. */ const struct got_error *got_diff_blob_file(struct got_blob_object *, FILE *, - off_t, const char *, FILE *, size_t, const char *, int, int, int, FILE *); + off_t, const char *, FILE *, int, size_t, const char *, int, int, int, + FILE *); /* * A callback function invoked to handle the differences between two blobs @@ -55,7 +54,7 @@ const struct got_error *got_diff_blob_file(struct got_ * the second blob contains content on the new side of the diff. * 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 + * The blob object argument for either blob may be NULL to indicate * that no content is present on its respective side of the diff. * File modes from relevant tree objects which contain the blobs may * also be passed. These will be zero if not available. blob - 13a1b0bf323c1c003dbb7b2473b2d1a20c202a43 blob + 500149f5a3358f499c291ab40acdc5142bf68184 --- lib/diff.c +++ lib/diff.c @@ -215,7 +215,7 @@ got_diff_blob(off_t **line_offsets, size_t *nlines, static const struct got_error * diff_blob_file(struct got_diffreg_result **resultp, struct got_blob_object *blob1, FILE *f1, off_t size1, const char *label1, - FILE *f2, size_t size2, const char *label2, int diff_context, + FILE *f2, int f2_exists, size_t size2, const char *label2, int diff_context, int ignore_whitespace, int force_text_diff, FILE *outfile) { const struct got_error *err = NULL, *free_err; @@ -234,7 +234,7 @@ diff_blob_file(struct got_diffreg_result **resultp, if (outfile) { fprintf(outfile, "blob - %s\n", label1 ? label1 : idstr1); fprintf(outfile, "file + %s\n", - f2 == NULL ? "/dev/null" : label2); + f2_exists ? label2 : "/dev/null"); } err = got_diffreg(&result, f1, f2, GOT_DIFF_ALGORITHM_PATIENCE, @@ -244,7 +244,7 @@ diff_blob_file(struct got_diffreg_result **resultp, if (outfile) { err = got_diffreg_output(NULL, NULL, result, - f1 != NULL, f2 != NULL, + blob1 != NULL, f2_exists, label2, /* show local file's path, not a blob ID */ label2, GOT_DIFF_OUTPUT_UNIDIFF, diff_context, outfile); @@ -265,15 +265,17 @@ done: const struct got_error * got_diff_blob_file(struct got_blob_object *blob1, FILE *f1, off_t size1, - const char *label1, FILE *f2, size_t size2, const char *label2, - int diff_context, int ignore_whitespace, int force_text_diff, FILE *outfile) + const char *label1, FILE *f2, int f2_exists, size_t size2, + const char *label2, int diff_context, int ignore_whitespace, + int force_text_diff, FILE *outfile) { - return diff_blob_file(NULL, blob1, f1, size1, label1, f2, size2, label2, - diff_context, ignore_whitespace, force_text_diff, outfile); + return diff_blob_file(NULL, blob1, f1, size1, label1, f2, f2_exists, + size2, label2, diff_context, ignore_whitespace, force_text_diff, + outfile); } static const struct got_error * -diff_added_blob(struct got_object_id *id, FILE *f, int fd, +diff_added_blob(struct got_object_id *id, FILE *f1, FILE *f2, int fd2, const char *label, mode_t mode, struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg) { @@ -285,10 +287,10 @@ diff_added_blob(struct got_object_id *id, FILE *f, int if (err) return err; - err = got_object_blob_open(&blob, repo, obj, 8192, fd); + err = got_object_blob_open(&blob, repo, obj, 8192, fd2); if (err) goto done; - err = cb(cb_arg, NULL, blob, NULL, f, NULL, id, + err = cb(cb_arg, NULL, blob, f1, f2, NULL, id, NULL, label, 0, mode, repo); done: got_object_close(obj); @@ -350,8 +352,8 @@ done: } static const struct got_error * -diff_deleted_blob(struct got_object_id *id, FILE *f, int fd, - const char *label, mode_t mode, struct got_repository *repo, +diff_deleted_blob(struct got_object_id *id, FILE *f1, int fd1, + FILE *f2, const char *label, mode_t mode, struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg) { const struct got_error *err; @@ -362,10 +364,10 @@ diff_deleted_blob(struct got_object_id *id, FILE *f, i if (err) return err; - err = got_object_blob_open(&blob, repo, obj, 8192, fd); + err = got_object_blob_open(&blob, repo, obj, 8192, fd1); if (err) goto done; - err = cb(cb_arg, blob, NULL, f, NULL, id, NULL, label, NULL, + err = cb(cb_arg, blob, NULL, f1, f2, id, NULL, label, NULL, mode, 0, repo); done: got_object_close(obj); @@ -375,9 +377,9 @@ done: } static const struct got_error * -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) +diff_added_tree(struct got_object_id *id, FILE *f1, FILE *f2, int fd2, + const char *label, struct got_repository *repo, got_diff_blob_cb cb, + void *cb_arg, int diff_content) { const struct got_error *err = NULL; struct got_object *treeobj = NULL; @@ -396,7 +398,7 @@ diff_added_tree(struct got_object_id *id, FILE *f, int if (err) goto done; - err = got_diff_tree(NULL, tree, NULL, f, -1, fd, NULL, label, + err = got_diff_tree(NULL, tree, f1, f2, -1, fd2, NULL, label, repo, cb, cb_arg, diff_content); done: if (tree) @@ -461,8 +463,8 @@ done: } static const struct got_error * -diff_deleted_tree(struct got_object_id *id, FILE *f, int fd, - const char *label, struct got_repository *repo, +diff_deleted_tree(struct got_object_id *id, FILE *f1, int fd1, + FILE *f2, const char *label, struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg, int diff_content) { const struct got_error *err; @@ -482,7 +484,7 @@ diff_deleted_tree(struct got_object_id *id, FILE *f, i if (err) goto done; - err = got_diff_tree(tree, NULL, f, NULL, fd, -1, label, NULL, + err = got_diff_tree(tree, NULL, f1, f2, fd1, -1, label, NULL, repo, cb, cb_arg, diff_content); done: if (tree) @@ -516,12 +518,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, fd1, label1, - repo, cb, cb_arg, diff_content); + err = diff_deleted_tree(&te1->id, f1, fd1, f2, + label1, repo, cb, cb_arg, diff_content); else { if (diff_content) err = diff_deleted_blob(&te1->id, f1, fd1, - label1, te1->mode, repo, cb, cb_arg); + f2, label1, te1->mode, repo, cb, cb_arg); else err = cb(cb_arg, NULL, NULL, NULL, NULL, &te1->id, NULL, label1, NULL, @@ -562,7 +564,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, int fd2, const char *label2, + struct got_tree_entry *te1, FILE *f1, FILE *f2, int fd2, const char *label2, struct got_repository *repo, got_diff_blob_cb cb, void *cb_arg, int diff_content) { @@ -573,11 +575,11 @@ diff_entry_new_old(struct got_tree_entry *te2, return NULL; if (S_ISDIR(te2->mode)) - return diff_added_tree(&te2->id, f2, fd2, label2, + return diff_added_tree(&te2->id, f1, f2, fd2, label2, repo, cb, cb_arg, diff_content); if (diff_content) - return diff_added_blob(&te2->id, f2, fd2, + return diff_added_blob(&te2->id, f1, f2, fd2, label2, te2->mode, repo, cb, cb_arg); return cb(cb_arg, NULL, NULL, NULL, NULL, NULL, &te2->id, @@ -690,8 +692,8 @@ got_diff_tree(struct got_tree_object *tree1, struct go return got_error_from_errno("asprintf"); } - err = diff_entry_new_old(te2, te, f2, fd2, l2, repo, - cb, cb_arg, diff_content); + err = diff_entry_new_old(te2, te, f1, f2, fd2, l2, + repo, cb, cb_arg, diff_content); if (err) break; } @@ -1086,9 +1088,9 @@ done: const struct got_error * got_diff_files(struct got_diffreg_result **resultp, - FILE *f1, const char *label1, FILE *f2, const char *label2, - int diff_context, int ignore_whitespace, int force_text_diff, - FILE *outfile) + FILE *f1, int f1_exists, const char *label1, FILE *f2, int f2_exists, + const char *label2, int diff_context, int ignore_whitespace, + int force_text_diff, FILE *outfile) { const struct got_error *err = NULL; struct got_diffreg_result *diffreg_result = NULL; @@ -1098,9 +1100,9 @@ got_diff_files(struct got_diffreg_result **resultp, if (outfile) { fprintf(outfile, "file - %s\n", - f1 == NULL ? "/dev/null" : label1); + f1_exists ? label1 : "/dev/null"); fprintf(outfile, "file + %s\n", - f2 == NULL ? "/dev/null" : label2); + f2_exists ? label2 : "/dev/null"); } err = got_diffreg(&diffreg_result, f1, f2, GOT_DIFF_ALGORITHM_PATIENCE, @@ -1110,7 +1112,7 @@ got_diff_files(struct got_diffreg_result **resultp, if (outfile) { err = got_diffreg_output(NULL, NULL, diffreg_result, - f1 != NULL, f2 != NULL, label1, label2, + f1_exists, f2_exists, label1, label2, GOT_DIFF_OUTPUT_UNIDIFF, diff_context, outfile); if (err) goto done; blob - 8121609c04313ea17c7ca2a5e48e324aed510006 blob + 186401edf2e3a5402baefc29f867ea4fcbf0b2db --- lib/diffreg.c +++ lib/diffreg.c @@ -90,8 +90,7 @@ const struct diff_config diff_config_no_algo = { }; const struct got_error * -got_diffreg_close(FILE *f1, char *p1, size_t size1, - FILE *f2, char *p2, size_t size2) +got_diffreg_close(char *p1, size_t size1, char *p2, size_t size2) { const struct got_error *err = NULL; @@ -99,10 +98,6 @@ got_diffreg_close(FILE *f1, char *p1, size_t size1, err = got_error_from_errno("munmap"); if (p2 && munmap(p2, size2) == -1 && err == NULL) err = got_error_from_errno("munmap"); - if (f1 && fclose(f1) == EOF && err == NULL) - err = got_error_from_errno("fclose"); - if (f2 && fclose(f2) == EOF && err == NULL) - err = got_error_from_errno("fclose"); return err; } @@ -186,7 +181,6 @@ got_diffreg(struct got_diffreg_result **diffreg_result const struct got_error *err = NULL; struct diff_config *cfg = NULL; char *p1 = NULL, *p2 = NULL; - int f1_created = 0, f2_created = 0; size_t size1, size2; struct diff_data d_left, d_right; struct diff_data *left, *right; @@ -209,23 +203,6 @@ got_diffreg(struct got_diffreg_result **diffreg_result if (err) goto done; - if (f1 == NULL) { - f1_created = 1; - f1 = got_opentemp(); - if (f1 == NULL) { - err = got_error_from_errno("got_opentemp"); - goto done; - } - } - if (f2 == NULL) { - f2_created = 1; - f2 = got_opentemp(); - if (f2 == NULL) { - err = got_error_from_errno("got_opentemp"); - goto done; - } - } - err = got_diff_prepare_file(f1, &p1, &size1, left, cfg, ignore_whitespace, force_text_diff); if (err) @@ -248,12 +225,8 @@ got_diffreg(struct got_diffreg_result **diffreg_result if (diffreg_result) { (*diffreg_result)->result = diff_result; - if (f1_created) - (*diffreg_result)->f1 = f1; (*diffreg_result)->map1 = p1; (*diffreg_result)->size1 = size1; - if (f2_created) - (*diffreg_result)->f2 = f2; (*diffreg_result)->map2 = p2; (*diffreg_result)->size2 = size2; } @@ -264,8 +237,7 @@ done: diff_data_free(right); } if (err) { - got_diffreg_close(f1_created ? f1 : NULL, p1, size1, - f2_created ? f2 : NULL, p2, size2); + got_diffreg_close(p1, size1, p2, size2); if (diffreg_result) { diff_data_free(left); diff_data_free(right); @@ -352,8 +324,7 @@ got_diffreg_result_free(struct got_diffreg_result *dif diff_result_free(diffreg_result->result); diff_data_free(&diffreg_result->left); diff_data_free(&diffreg_result->right); - err = got_diffreg_close(diffreg_result->f1, diffreg_result->map1, - diffreg_result->size1, diffreg_result->f2, + err = got_diffreg_close(diffreg_result->map1, diffreg_result->size1, diffreg_result->map2, diffreg_result->size2); free(diffreg_result); return err; @@ -364,8 +335,8 @@ got_diffreg_result_free_left(struct got_diffreg_result { diff_data_free(&diffreg_result->left); memset(&diffreg_result->left, 0, sizeof(diffreg_result->left)); - return got_diffreg_close(diffreg_result->f1, diffreg_result->map1, - diffreg_result->size1, NULL, NULL, 0); + return got_diffreg_close(diffreg_result->map1, diffreg_result->size1, + NULL, 0); } const struct got_error * @@ -373,6 +344,6 @@ got_diffreg_result_free_right(struct got_diffreg_resul { diff_data_free(&diffreg_result->right); memset(&diffreg_result->right, 0, sizeof(diffreg_result->right)); - return got_diffreg_close(NULL, NULL, 0, diffreg_result->f2, - diffreg_result->map2, diffreg_result->size2); + return got_diffreg_close(NULL, 0, diffreg_result->map2, + diffreg_result->size2); } blob - 6262295fc0820142511469142f7dfe853a485953 blob + 0157c869fff0c97a45ce1c6fc4f72dcca8c9f2b6 --- lib/got_lib_diff.h +++ lib/got_lib_diff.h @@ -30,10 +30,8 @@ enum got_diff_output_format { struct got_diffreg_result { struct diff_result *result; - FILE *f1; char *map1; size_t size1; - FILE *f2; char *map2; size_t size2; struct diff_data left; @@ -59,11 +57,10 @@ const struct got_error *got_diffreg_result_free_left( struct got_diffreg_result *); const struct got_error *got_diffreg_result_free_right( struct got_diffreg_result *); -const struct got_error *got_diffreg_close(FILE *, char *, size_t, - FILE *, char *, size_t); +const struct got_error *got_diffreg_close(char *, size_t, char *, size_t); const struct got_error *got_merge_diff3(int *, int, FILE *, FILE *, FILE *, const char *, const char *, const char *, enum got_diff_algorithm); const struct got_error *got_diff_files(struct got_diffreg_result **, FILE *, - const char *, FILE *, const char *, int, int, int, FILE *); + int, const char *, FILE *, int, const char *, int, int, int, FILE *); blob - 18f13a154b509692bfadad8d3d5873d52e5bd66b blob + c6a2aa553557aa15a464291525d9d8c0ee48bcb8 --- lib/worktree.c +++ lib/worktree.c @@ -4546,8 +4546,8 @@ create_patched_content(char **path_outfile, int revers if (err) goto done; - err = got_diff_files(&diffreg_result, f1, id_str, f2, path2, 3, 0, 1, - NULL); + err = got_diff_files(&diffreg_result, f1, 1, id_str, f2, 1, path2, + 3, 0, 1, NULL); if (err) goto done; @@ -8361,7 +8361,7 @@ create_unstaged_content(char **path_unstaged_content, if (err) goto done; - err = got_diff_files(&diffreg_result, f1, label1, f2, + err = got_diff_files(&diffreg_result, f1, 1, label1, f2, 1, path2, 3, 0, 1, NULL); if (err) goto done;