Commit Diff

Commit:
3a975a9fcbb052d6cbf4d9bc062683250a23f466
From:
Stefan Sperling <stsp@stsp.name>
Date:
Message:
fix tempfile handling in gotd's got_object_raw_open() There was a bug where we reused a file stored in *outfd from a previous call, resulting in a raw object backed by a file but with tempfile_idx -1. This then caused bad confusion during deltification. Fix this by passing tempfd to read_raw() functions and only setting *outfd in case it is actually required, and returning tempfd to the repository tempfile pool otherwise. ok tracey
Actions:
Patch | Tree
commit - dc07f76c4f8d4418ce30fd4eb087756fe9fb6ded
commit + 3a975a9fcbb052d6cbf4d9bc062683250a23f466
blob - 312719b3bcc84abe056dab97015657c4a30442f6
blob + 40afcdfffec70644278cf7d34e73724d4932e5ff
--- lib/object_open_io.c
+++ lib/object_open_io.c
@@ -250,7 +250,7 @@ got_object_raw_open(struct got_raw_object **obj, int *
 {
 	const struct got_error *err = NULL;
 	struct got_packidx *packidx = NULL;
-	int idx, tempfile_idx = -1;
+	int idx, tempfd, tempfile_idx;
 	uint8_t *outbuf = NULL;
 	off_t size = 0;
 	size_t hdrlen = 0;
@@ -261,21 +261,10 @@ got_object_raw_open(struct got_raw_object **obj, int *
 		(*obj)->refcnt++;
 		return NULL;
 	}
-
-	if (*outfd == -1) {
-		int tempfd;
-
-		err = got_repo_temp_fds_get(&tempfd, &tempfile_idx, repo);
-		if (err)
-			return err;
 
-		/* Duplicate tempfile descriptor to allow use of fdopen(3). */
-		*outfd = dup(tempfd);
-		if (*outfd == -1) {
-			got_repo_temp_fds_put(tempfile_idx, repo);
-			return got_error_from_errno("dup");
-		}
-	}
+	err = got_repo_temp_fds_get(&tempfd, &tempfile_idx, repo);
+	if (err)
+		return err;
 
 	err = got_repo_search_packidx(&packidx, &idx, repo, id);
 	if (err == NULL) {
@@ -294,7 +283,7 @@ got_object_raw_open(struct got_raw_object **obj, int *
 				goto done;
 		}
 		err = read_packed_object_raw(&outbuf, &size, &hdrlen,
-		    *outfd, pack, packidx, idx, id);
+		    tempfd, pack, packidx, idx, id);
 		if (err)
 			goto done;
 	} else if (err->code == GOT_ERR_NO_OBJ) {
@@ -304,11 +293,28 @@ got_object_raw_open(struct got_raw_object **obj, int *
 		if (err)
 			goto done;
 		err = got_object_read_raw(&outbuf, &size, &hdrlen,
-		    GOT_DELTA_RESULT_SIZE_CACHED_MAX, *outfd, id, fd);
+		    GOT_DELTA_RESULT_SIZE_CACHED_MAX, tempfd, id, fd);
 		if (close(fd) == -1 && err == NULL)
 			err = got_error_from_errno("close");
 		if (err)
+			goto done;
+	}
+
+	if (outbuf == NULL) {
+		if (*outfd != -1) {
+			err = got_error_msg(GOT_ERR_NOT_IMPL, "bad outfd");
+			goto done;
+		}
+
+		/*
+		 * Duplicate tempfile descriptor to allow use of
+		 * fdopen(3) inside got_object_raw_alloc().
+		 */
+		*outfd = dup(tempfd);
+		if (*outfd == -1) {
+			err = got_error_from_errno("dup");
 			goto done;
+		}
 	}
 
 	err = got_object_raw_alloc(obj, outbuf, outfd,
@@ -330,12 +336,24 @@ done:
 			*obj = NULL;
 		}
 		free(outbuf);
-		if (tempfile_idx != -1)
-			got_repo_temp_fds_put(tempfile_idx, repo);
+		got_repo_temp_fds_put(tempfile_idx, repo);
+		if (*outfd != -1) {
+			close(*outfd);
+			*outfd = -1;
+		}
 	} else {
-		(*obj)->tempfile_idx = tempfile_idx;
-		(*obj)->close_cb = put_raw_object_tempfile;
-		(*obj)->close_arg = repo;
+		if (((*obj)->f == NULL && (*obj)->fd == -1)) {
+			/* This raw object is not backed by a file. */
+			got_repo_temp_fds_put(tempfile_idx, repo);
+			if (*outfd != -1) {
+				close(*outfd);
+				*outfd = -1;
+			}
+		} else {
+			(*obj)->tempfile_idx = tempfile_idx;
+			(*obj)->close_cb = put_raw_object_tempfile;
+			(*obj)->close_arg = repo;
+		}
 	}
 	return err;
 }