From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 06 Apr 2020 14:45:37 GMT
Subject: Re: [PATCH v4 07/23] sequencer: configurably warn on non-existent files
Message-ID: <02110ed3-19c2-9074-2b30-90a4b289b0a8@gmail.com>
In-Reply-To: <2e7922b259851091b9014c761d340ff1a73a04e4.1585962672.git.liu.denton@gmail.com>

Hi Denton

On 04/04/2020 02:11, Denton Liu wrote:
> In the future, we plan on externing read_oneliner(). Future users of
> read_oneliner() will want the ability to output warnings in the event
> that the `path` doesn't exist. Introduce the
> `READ_ONELINER_WARN_MISSING` flag which, if active, would issue a
> warning when a file doesn't exist by skipping the `!file_exists()` check
> and letting `strbuf_read_file()` handle that case.

Now that you've updated the function in a previous patch the 
file_exists() check referred to in the message no longer exists

> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>   sequencer.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 6c26d61670..b771e8f4ca 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -420,6 +420,7 @@ static int write_message(const void *buf, size_t len, const char *filename,
>   }
>   
>   #define READ_ONELINER_SKIP_IF_EMPTY (1 << 0)
> +#define READ_ONELINER_WARN_MISSING (1 << 1)
>   
>   /*
>    * Resets a strbuf then reads a file that was presumably written by a shell
> @@ -435,7 +436,8 @@ static int read_oneliner(struct strbuf *buf,
>   {
>   	strbuf_reset(buf);
>   	if (strbuf_read_file(buf, path, 0) < 0) {
> -		if (errno != ENOENT && errno != ENOTDIR)
> +		if ((flags & READ_ONELINER_WARN_MISSING) ||
> +				(errno != ENOENT && errno != ENOTDIR))

We don't need the extra indentation for '(errno != ...', it should align 
with the second '(' on the line above I think

Best Wishes

Phillip

>   			warning_errno(_("could not read '%s'"), path);
>   		return 0;
>   	}
>