Got Mirror

Commit Diff

Commit:: 04b2c111f2cee68f0aa07855fcb7329a4913050b
From:: Omar Polo <op@omarpolo.com>
Via:: Thomas Adam <thomas@xteddy.org>
Date:: Tue May 16 11:05:39 2023 UTC
Message:: simplify got_gotweb_openfile/flushfile Return only the FILE *, and require only that to close subsequently. Having to manage the file descriptor too makes using these functions unnecessary complex. millert@ agrees
Actions:: Patch | Tree
commit - f292b27176775cc4061f3da0be7fe0efcaf0325c
commit + 04b2c111f2cee68f0aa07855fcb7329a4913050b
blob - 49849c1f11676fb3f5efd0fbc0d00c2eae817bec
blob + 82e38605c1ffbff88bf742a36e48bb502581d804
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -48,41 +48,40 @@ static const struct got_error *got_get_repo_commit(str
     struct repo_commit *, struct got_commit_object *, struct got_reflist_head *,
     struct got_object_id *);
 static const struct got_error *got_gotweb_dupfd(int *, int *);
-static const struct got_error *got_gotweb_openfile(FILE **, int *, int *);
+static const struct got_error *got_gotweb_openfile(FILE **, int *);
 static const struct got_error *got_gotweb_blame_cb(void *, int, int,
     struct got_commit_object *,struct got_object_id *);
 
 const struct got_error *
-got_gotweb_flushfile(FILE *f, int fd)
+got_gotweb_flushfile(FILE *f)
 {
 	if (fseek(f, 0, SEEK_SET) == -1)
 		return got_error_from_errno("fseek");
 
-	if (ftruncate(fd, 0) == -1)
+	if (ftruncate(fileno(f), 0) == -1)
 		return got_error_from_errno("ftruncate");
 
-	if (fsync(fd) == -1)
+	if (fsync(fileno(f)) == -1)
 		return got_error_from_errno("fsync");
 
-	if (f && fclose(f) == EOF)
+	if (fclose(f) == EOF)
 		return got_error_from_errno("fclose");
 
-	if (fd != -1 && close(fd) != -1)
-		return got_error_from_errno("close");
-
 	return NULL;
 }
 
 static const struct got_error *
-got_gotweb_openfile(FILE **f, int *priv_fd, int *fd)
+got_gotweb_openfile(FILE **f, int *priv_fd)
 {
-	*fd = dup(*priv_fd);
-	if (*fd == -1)
+	int fd;
+
+	fd = dup(*priv_fd);
+	if (fd == -1)
 		return got_error_from_errno("dup");
 
-	*f = fdopen(*fd, "w+");
+	*f = fdopen(fd, "w+");
 	if (*f == NULL) {
-		close(*fd);
+		close(fd);
 		return got_error(GOT_ERR_PRIVSEP_NO_FD);
 	}
 
@@ -1012,8 +1011,7 @@ got_output_file_blame(struct request *c, got_render_bl
 	struct got_blob_object *blob = NULL;
 	char *path = NULL, *in_repo_path = NULL;
 	struct blame_cb_args bca;
-	int i, obj_type, fd1 = -1, fd2 = -1, fd3 = -1, fd4 = -1, fd5 = -1;
-	int fd6 = -1;
+	int i, obj_type, fd2 = -1, fd3 = -1, fd4 = -1;
 	off_t filesize;
 	FILE *f1 = NULL, *f2 = NULL;
 
@@ -1054,7 +1052,7 @@ got_output_file_blame(struct request *c, got_render_bl
 		goto done;
 	}
 
-	error = got_gotweb_openfile(&bca.f, &c->priv_fd[BLAME_FD_1], &fd1);
+	error = got_gotweb_openfile(&bca.f, &c->priv_fd[BLAME_FD_1]);
 	if (error)
 		goto done;
 
@@ -1098,11 +1096,11 @@ got_output_file_blame(struct request *c, got_render_bl
 	if (error)
 		goto done;
 
-	error = got_gotweb_openfile(&f1, &c->priv_fd[BLAME_FD_5], &fd5);
+	error = got_gotweb_openfile(&f1, &c->priv_fd[BLAME_FD_5]);
 	if (error)
 		goto done;
 
-	error = got_gotweb_openfile(&f2, &c->priv_fd[BLAME_FD_6], &fd6);
+	error = got_gotweb_openfile(&f2, &c->priv_fd[BLAME_FD_6]);
 	if (error)
 		goto done;
 
@@ -1127,20 +1125,17 @@ done:
 	if (fd4 != -1 && close(fd4) == -1 && error == NULL)
 		error = got_error_from_errno("close");
 	if (bca.f) {
-		const struct got_error *bca_err =
-		    got_gotweb_flushfile(bca.f, fd1);
+		const struct got_error *bca_err = got_gotweb_flushfile(bca.f);
 		if (error == NULL)
 			error = bca_err;
 	}
 	if (f1) {
-		const struct got_error *f1_err =
-		    got_gotweb_flushfile(f1, fd5);
+		const struct got_error *f1_err = got_gotweb_flushfile(f1);
 		if (error == NULL)
 			error = f1_err;
 	}
 	if (f2) {
-		const struct got_error *f2_err =
-		    got_gotweb_flushfile(f2, fd6);
+		const struct got_error *f2_err = got_gotweb_flushfile(f2);
 		if (error == NULL)
 			error = f2_err;
 	}
@@ -1157,7 +1152,7 @@ done:
 }
 
 const struct got_error *
-got_open_diff_for_output(FILE **fp, int *fd, struct request *c)
+got_open_diff_for_output(FILE **fp, struct request *c)
 {
 	const struct got_error *error = NULL;
 	struct transport *t = c->t;
@@ -1166,22 +1161,21 @@ got_open_diff_for_output(FILE **fp, int *fd, struct re
 	struct got_object_id *id1 = NULL, *id2 = NULL;
 	struct got_reflist_head refs;
 	FILE *f1 = NULL, *f2 = NULL, *f3 = NULL;
-	int obj_type, fd1, fd2, fd3, fd4 = -1, fd5 = -1;
+	int obj_type, fd4 = -1, fd5 = -1;
 
 	*fp = NULL;
-	*fd = -1;
 
 	TAILQ_INIT(&refs);
 
-	error = got_gotweb_openfile(&f1, &c->priv_fd[DIFF_FD_1], &fd1);
+	error = got_gotweb_openfile(&f1, &c->priv_fd[DIFF_FD_1]);
 	if (error)
 		return error;
 
-	error = got_gotweb_openfile(&f2, &c->priv_fd[DIFF_FD_2], &fd2);
+	error = got_gotweb_openfile(&f2, &c->priv_fd[DIFF_FD_2]);
 	if (error)
 		return error;
 
-	error = got_gotweb_openfile(&f3, &c->priv_fd[DIFF_FD_3], &fd3);
+	error = got_gotweb_openfile(&f3, &c->priv_fd[DIFF_FD_3]);
 	if (error)
 		return error;
 
@@ -1251,7 +1245,6 @@ got_open_diff_for_output(FILE **fp, int *fd, struct re
 	}
 
 	*fp = f3;
-	*fd = fd3;
 
 done:
 	if (fd4 != -1 && close(fd4) == -1 && error == NULL)
@@ -1259,21 +1252,18 @@ done:
 	if (fd5 != -1 && close(fd5) == -1 && error == NULL)
 		error = got_error_from_errno("close");
 	if (f1) {
-		const struct got_error *f1_err =
-		    got_gotweb_flushfile(f1, fd1);
+		const struct got_error *f1_err = got_gotweb_flushfile(f1);
 		if (error == NULL)
 			error = f1_err;
 	}
 	if (f2) {
-		const struct got_error *f2_err =
-		    got_gotweb_flushfile(f2, fd2);
+		const struct got_error *f2_err = got_gotweb_flushfile(f2);
 		if (error == NULL)
 			error = f2_err;
 	}
 	if (error && f3) {
-		got_gotweb_flushfile(f3, fd3);
+		got_gotweb_flushfile(f3);
 		*fp = NULL;
-		*fd = -1;
 	}
 	got_ref_list_free(&refs);
 	free(id1);
blob - 7adff244e79d7255b82e75217b2f4ef08aea0a1c
blob + 73dbb1e0e2e499ffe8416b6d4afbca31e255970f
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -287,7 +287,7 @@ gotweb_process_request(struct request *c)
 			log_warnx("%s: %s", __func__, error->msg);
 			goto err;
 		}
-		error = got_open_diff_for_output(&c->t->fp, &c->t->fd, c);
+		error = got_open_diff_for_output(&c->t->fp, c);
 		if (error) {
 			log_warnx("%s: %s", __func__, error->msg);
 			goto err;
@@ -742,11 +742,10 @@ gotweb_free_transport(struct transport *t)
 	if (t->blob)
 		got_object_blob_close(t->blob);
 	if (t->fp) {
-		err = got_gotweb_flushfile(t->fp, t->fd);
+		err = got_gotweb_flushfile(t->fp);
 		if (err)
 			log_warnx("%s: got_gotweb_flushfile failure: %s",
 			    __func__, err->msg);
-		t->fd = -1;
 	}
 	if (t->fd != -1)
 		close(t->fd);
blob - 9e8e0a7344f99e5a308fc5d0932d6f1dc7d81e61
blob + 6143eaf2f67efd948d7df9e1ce6ed0c28f584cce
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -508,15 +508,14 @@ int fcgi_printf(struct request *, const char *, ...)
 int fcgi_gen_binary_response(struct request *, const uint8_t *, int);
 
 /* got_operations.c */
-const struct got_error *got_gotweb_flushfile(FILE *, int);
+const struct got_error *got_gotweb_flushfile(FILE *);
 const struct got_error *got_get_repo_owner(char **, struct request *);
 const struct got_error *got_get_repo_age(time_t *, struct request *,
     const char *);
 const struct got_error *got_get_repo_commits(struct request *, int);
 const struct got_error *got_get_repo_tags(struct request *, int);
 const struct got_error *got_get_repo_heads(struct request *);
-const struct got_error *got_open_diff_for_output(FILE **, int *,
-    struct request *);
+const struct got_error *got_open_diff_for_output(FILE **, struct request *);
 int got_output_repo_tree(struct request *,
     int (*)(struct template *, struct got_tree_entry *));
 const struct got_error *got_open_blob_for_output(struct got_blob_object **,