From: Phillip Wood <phillip.wood123@gmail.com>
Date: Thu, 02 Apr 2020 13:39:26 GMT
Subject: Re: [PATCH v3 05/19] sequencer: configurably warn on non-existent files
Message-ID: <5c81eca7-0990-1c60-7f9f-04bce05be3ba@gmail.com>
In-Reply-To: <e42f4bdd4d763be4062f12fa6f1481849e87a0ad.1584782450.git.liu.denton@gmail.com>

Hi Denton

On 21/03/2020 09:21, 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_NON_EXISTENCE`

This is a rather cumbersome name, I think Junio's original suggestion of 
READ_ONELINER_WARN_MISSING is nicer

> 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.
> 
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>   sequencer.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index abb2a21e9d..92e8d38290 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_NON_EXISTENCE (1 << 1)
>   
>   /*
>    * Reads a file that was presumably written by a shell script, i.e. with an
> @@ -436,7 +437,7 @@ static int read_oneliner(struct strbuf *buf,
>   	int ret = 0;
>   	struct strbuf file_buf = STRBUF_INIT;
>   
> -	if (!file_exists(path))
> +	if (!(flags & READ_ONELINER_WARN_NON_EXISTENCE) && !file_exists(path))
>   		return 0;

This isn't the fault of you patch but it would be more efficient to do 
the read and then check for ENOENT/ENOTDIR rather than calling 
file_exists() first

Best Wishes

Phillip

>   	if (strbuf_read_file(&file_buf, path, 0) < 0) {
>