From: René Scharfe <l.s.r@web.de>
Date: Wed, 01 Nov 2017 17:09:49 GMT
Subject: Re: [PATCH 2/2] sequencer.c: check return value of close() in rewrite_file()
Message-ID: <44fa927f-d4bc-9396-0a50-bc2366d4fadc@web.de>
In-Reply-To: <06c33d3cfa35c0524ede2970ee3169d6c62eb5c1.1509547231.git.simon@ruderich.org>

Am 01.11.2017 um 15:45 schrieb Simon Ruderich:
> Not checking close(2) can hide errors as not all errors are reported
> during the write(2).
> 
> Signed-off-by: Simon Ruderich <simon@ruderich.org>
> ---
> 
> On Wed, Nov 01, 2017 at 02:00:11PM +0100, René Scharfe wrote:
>> Most calls are not checked, but that doesn't necessarily mean they need
>> to (or should) stay that way.  The Linux man-page of close(2) spends
>> multiple paragraphs recommending to check its return value..  Care to
>> send a follow-up patch?
> 
> Hello,
> 
> Sure, here is it.
> 
> Regards
> Simon
> 
>   sequencer.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index f93b60f61..e0cc2f777 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2673,7 +2673,8 @@ static int rewrite_file(const char *path, const char *buf, size_t len)
>   		return error_errno(_("could not open '%s' for writing"), path);
>   	if (write_in_full(fd, buf, len) < 0)
>   		rc = error_errno(_("could not write to '%s'"), path);
> -	close(fd);
> +	if (close(fd) && !rc)
> +		rc = error_errno(_("could not close '%s'"), path);
>   	return rc;
>   }
>   
> 

Looks good to me, thank you!

René