Jeff King wrote:
> When we are streaming an index blob to disk, we store the
> error from stream_blob_to_fd in the "result" variable, and
> then immediately overwrite that with the return value of
> "close".
Good catch.
[...]
> --- a/entry.c
> +++ b/entry.c
> @@ -126,8 +126,10 @@ static int streaming_write_entry(struct cache_entry *ce, char *path,
> fd = open_output_fd(path, ce, to_tempfile);
> if (0 <= fd) {
> result = stream_blob_to_fd(fd, ce->sha1, filter, 1);
> - *fstat_done = fstat_output(fd, state, statbuf);
> - result = close(fd);
> + if (!result) {
> + *fstat_done = fstat_output(fd, state, statbuf);
> + result = close(fd);
> + }
Should this do something like
{
int fd, result = 0;
fd = open_output_fd(path, ce, to_tempfile);
if (fd < 0)
return -1;
result = stream_blob_to_fd(fd, ce->sha1, filter, 1);
if (result)
goto close_fd;
*fstat_done = fstat_output(fd, state, statbuf);
close_fd:
result |= close(fd);
unlink_path:
if (result)
unlink(path);
return result;
}
to avoid leaking the file descriptor?
> @@ -31,4 +40,20 @@ test_expect_success 'streaming a corrupt blob fails' '
> )
> '
>
> +test_expect_success 'read-tree -u detects bit-errors in blobs' '
> + (
> + cd bit-error &&
> + rm content.t &&
> + test_must_fail git read-tree --reset -u FETCH_HEAD
> + )
Makes sense. Might make sense to use "rm -f" instead of "rm" to avoid
failures if content.t is removed already.
> +'
> +
> +test_expect_success 'read-tree -u detects missing objects' '
> + (
> + cd missing &&
> + rm content.t &&
Especially here.
Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html