util: Don't swear on interrupted close syscall

Submitted by Cyrill Gorcunov on May 2, 2017, 9:16 p.m.

Details

Message ID 1493759813-15786-1-git-send-email-gorcunov@virtuozzo.com
State Rejected
Series "util: Don't swear on interrupted close syscall"
Headers show

Commit Message

Cyrill Gorcunov May 2, 2017, 9:16 p.m.
In case of EINTR here the descriptor will be
closed anyway.

http://lkml.indiana.edu/hypermail/linux/kernel/0509.1/0877.html

Signed-off-by: Cyrill Gorcunov <gorcunov@virtuozzo.com>
---
 criu/util.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/util.c b/criu/util.c
index cb1e8fd..f5ed388 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -183,8 +183,12 @@  int close_safe(int *fd)
 		ret = close(*fd);
 		if (!ret)
 			*fd = -1;
-		else
-			pr_perror("Unable to close fd %d", *fd);
+		else {
+			if (errno != EINTR)
+				pr_perror("Unable to close fd %d", *fd);
+			else
+				ret = 0, *fd = -1;
+		}
 	}
 
 	return ret;

Comments

Pavel Emelianov May 4, 2017, 1:33 p.m.
On 05/03/2017 12:16 AM, Cyrill Gorcunov wrote:
> In case of EINTR here the descriptor will be
> closed anyway.

We do we have this EINTR here at all?

> http://lkml.indiana.edu/hypermail/linux/kernel/0509.1/0877.html
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@virtuozzo.com>
> ---
>  criu/util.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/criu/util.c b/criu/util.c
> index cb1e8fd..f5ed388 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -183,8 +183,12 @@ int close_safe(int *fd)
>  		ret = close(*fd);
>  		if (!ret)
>  			*fd = -1;
> -		else
> -			pr_perror("Unable to close fd %d", *fd);
> +		else {
> +			if (errno != EINTR)
> +				pr_perror("Unable to close fd %d", *fd);
> +			else
> +				ret = 0, *fd = -1;
> +		}
>  	}
>  
>  	return ret;
>
Kirill Gorkunov May 4, 2017, 1:36 p.m.
On Thu, May 04, 2017 at 04:33:56PM +0300, Pavel Emelyanov wrote:
> On 05/03/2017 12:16 AM, Cyrill Gorcunov wrote:
> > In case of EINTR here the descriptor will be
> > closed anyway.
> 
> We do we have this EINTR here at all?

Any syscall may exit with EINTR, except in case of close it is
special because file will be closed anyway.
Andrey Vagin May 5, 2017, 5:51 p.m.
On Thu, May 04, 2017 at 04:36:24PM +0300, Cyrill Gorcunov wrote:
> On Thu, May 04, 2017 at 04:33:56PM +0300, Pavel Emelyanov wrote:
> > On 05/03/2017 12:16 AM, Cyrill Gorcunov wrote:
> > > In case of EINTR here the descriptor will be
> > > closed anyway.
> > 
> > We do we have this EINTR here at all?
> 
> Any syscall may exit with EINTR, except in case of close it is
> special because file will be closed anyway.

But there should be a reason for this EINTR. Do you have a log when
close returns EINTR in criu?

> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Kirill Gorkunov May 5, 2017, 6:06 p.m.
On Fri, May 05, 2017 at 10:51:49AM -0700, Andrei Vagin wrote:
> On Thu, May 04, 2017 at 04:36:24PM +0300, Cyrill Gorcunov wrote:
> > On Thu, May 04, 2017 at 04:33:56PM +0300, Pavel Emelyanov wrote:
> > > On 05/03/2017 12:16 AM, Cyrill Gorcunov wrote:
> > > > In case of EINTR here the descriptor will be
> > > > closed anyway.
> > > 
> > > We do we have this EINTR here at all?
> > 
> > Any syscall may exit with EINTR, except in case of close it is
> > special because file will be closed anyway.
> 
> But there should be a reason for this EINTR. Do you have a log when
> close returns EINTR in criu?

No, i don't have log.
Andrey Vagin May 5, 2017, 6:38 p.m.
On Fri, May 05, 2017 at 09:06:53PM +0300, Cyrill Gorcunov wrote:
> On Fri, May 05, 2017 at 10:51:49AM -0700, Andrei Vagin wrote:
> > On Thu, May 04, 2017 at 04:36:24PM +0300, Cyrill Gorcunov wrote:
> > > On Thu, May 04, 2017 at 04:33:56PM +0300, Pavel Emelyanov wrote:
> > > > On 05/03/2017 12:16 AM, Cyrill Gorcunov wrote:
> > > > > In case of EINTR here the descriptor will be
> > > > > closed anyway.
> > > > 
> > > > We do we have this EINTR here at all?
> > > 
> > > Any syscall may exit with EINTR, except in case of close it is
> > > special because file will be closed anyway.
> > 
> > But there should be a reason for this EINTR. Do you have a log when
> > close returns EINTR in criu?
> 
> No, i don't have log.

Why did you decide to make this patch?;)
Kirill Gorkunov May 5, 2017, 6:42 p.m.
On Fri, May 05, 2017 at 11:38:51AM -0700, Andrei Vagin wrote:
> > No, i don't have log.
> 
> Why did you decide to make this patch?;)

Doesn't matter, drop it please.