Re: [PATCH v15 22/27] bisect--helper: `bisect_log` shell function in C

[Stephan Beyer]

Hi,

On 10/14/2016 04:14 PM, Pranit Bauva wrote:
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 493034c..c18ca07 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -858,6 +858,23 @@ static int bisect_state(struct bisect_terms *terms, 
> const char **argv,
>       return -1;
>  }
>  
> +static int bisect_log(void)
> +{
> +     int fd, status;
> +     fd = open(git_path_bisect_log(), O_RDONLY);
> +     if (fd < 0)
> +             return -1;
> +
> +     status = copy_fd(fd, 1);

Perhaps

        status = copy_fd(fd, STDOUT_FILENO);

> +     if (status) {
> +             close(fd);
> +             return -1;
> +     }
> +
> +     close(fd);
> +     return status;
> +}

That's weird.
Either get rid of the if() and actually use status:

        status = copy_fd(fd, STDOUT_FILENO);

        close(fd);
        return status ? -1 : 0;

or get rid of status and use the if:

        if (copy_fd(fd, STDOUT_FILENO)) {
                close(fd);
                return -1;
        }

        close(fd);
        return 0;

I'd recommend the shorter variant ;)

~Stephan