Download raw body.
fix tempfile handling in gotd's got_object_raw_open()
There is a bug where we reuse a file stored in *outfd from a previous call, resulting in a raw object backed by a file but with tempfile_idx -1. This causes 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. With this, a final "input/output" error I see while cloning ports.git from gotd disappears. I attributed this error to having egdb attached to gotd at first, but that was wrong. The problem seen during deltification was that an object ended up being backed by the wrong file, namely a slightly smaller file which contained the delta-base instead of the object. gotd would then an attempt to read beyond the end of this file. diff 60c140aebbd1069094e75d8358cb305bc305b7f8 639adcb774ebc1459df58b791ffcc037002de351 commit - 60c140aebbd1069094e75d8358cb305bc305b7f8 commit + 639adcb774ebc1459df58b791ffcc037002de351 blob - 312719b3bcc84abe056dab97015657c4a30442f6 blob + dc5a7253bb299918f457cf7b0a2116b13328d7b1 --- 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; @@ -262,21 +262,10 @@ got_object_raw_open(struct got_raw_object **obj, int * return NULL; } - if (*outfd == -1) { - int tempfd; + err = got_repo_temp_fds_get(&tempfd, &tempfile_idx, repo); + if (err) + return err; - 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_search_packidx(&packidx, &idx, repo, id); if (err == NULL) { struct got_pack *pack = 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,13 +293,30 @@ 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, GOT_DELTA_RESULT_SIZE_CACHED_MAX, hdrlen, size); if (err) @@ -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; }
fix tempfile handling in gotd's got_object_raw_open()