Re: [PATCH v2 1/3] bisect: read run output from the open descriptor

[Junio C Hamano <gitster@pobox.com>]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> "git bisect run" redirects each step's output into BISECT_RUN, then
> prints it back by reopening the file by name. Read it from the already
> open descriptor instead; this behaves the same and no longer needs the
> file to be reachable by name.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>  builtin/bisect.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/bisect.c b/builtin/bisect.c
> index 798e28f501..69ea14b1b6 100644
> --- a/builtin/bisect.c
> +++ b/builtin/bisect.c
> @@ -178,17 +178,13 @@ static int append_to_file(const char *path, const char *format, ...)
>  	return res;
>  }
>  
> -static int print_file_to_stdout(const char *path)
> +static int print_fd_to_stdout(int fd)
>  {
> -	int fd = open(path, O_RDONLY);
> -	int ret = 0;
> -
> -	if (fd < 0)
> -		return error_errno(_("cannot open file '%s' for reading"), path);
> +	if (lseek(fd, 0, SEEK_SET) < 0)
> +		return error_errno(_("failed to rewind BISECT_RUN output"));

OK.  So, instead of the usual 'O_CREAT | O_WRONLY', you use 'O_RDWR'
instead, so that we can switch from writing to reading at this
point.  That makes sense.

I wonder if there are cases where we somehow fail to seek, and
yet are still able to open the path for reading and copy the
data successfully.  If such a case is common, this change
would be a regression, but I cannot offhand think of a
scenario where that would occur.

Will queue.

Thanks.