Re: [PATCH v2 10/21] log: report errno on failure to fopen() a file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Duy,

On Wed, 3 May 2017, Nguyễn Thái Ngọc Duy wrote:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>

I do think that this merits a one-line explanation why you do not use
fopen_or_warn() here.

> diff --git a/builtin/log.c b/builtin/log.c
> index b3b10cc1ed..26d6a3cf14 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -858,7 +858,8 @@ static int open_next_file(struct commit *commit, const char *subject,
>  		printf("%s\n", filename.buf + outdir_offset);
>  
>  	if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL)
> -		return error(_("Cannot open patch file %s"), filename.buf);
> +		return error_errno(_("Cannot open patch file %s"),
> +				   filename.buf);
>  
>  	strbuf_release(&filename);

The patches before and after this one all convert fopen() to
fopen_or_warn(). If you *must* separate those into individual patches
(which just makes this patch series unnecessarily bloated, if you ask me),
you should at least group them a bit better so that the patch series tells
more of a consistent story rather than jumping back and forth like a
hyperactive four-year-old telling you about her latest adventure.

Ciao,
Dscho

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]