From owner-freebsd-arch@FreeBSD.ORG Thu Aug 2 11:52:18 2012 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id B99911065672; Thu, 2 Aug 2012 11:52:18 +0000 (UTC) (envelope-from listlog2011@gmail.com) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 857C58FC08; Thu, 2 Aug 2012 11:52:18 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q72BqEXl040217; Thu, 2 Aug 2012 11:52:15 GMT (envelope-from listlog2011@gmail.com) Message-ID: <501A69EB.9000701@gmail.com> Date: Thu, 02 Aug 2012 19:52:11 +0800 From: David Xu User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20120713 Thunderbird/14.0 MIME-Version: 1.0 To: Konstantin Belousov References: <5018992C.8000207@freebsd.org> <20120801071934.GJ2676@deviant.kiev.zoral.com.ua> <20120801183240.K1291@besplex.bde.org> <20120801162836.GO2676@deviant.kiev.zoral.com.ua> <20120802040542.G2978@besplex.bde.org> <20120802100240.GV2676@deviant.kiev.zoral.com.ua> In-Reply-To: <20120802100240.GV2676@deviant.kiev.zoral.com.ua> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: arch@freebsd.org, David Xu Subject: Re: short read/write and error code X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: davidxu@freebsd.org List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Aug 2012 11:52:18 -0000 On 2012/8/2 18:02, Konstantin Belousov wrote: > diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c > index 338256c..1a61b93 100644 > --- a/sys/kern/sys_pipe.c > +++ b/sys/kern/sys_pipe.c > @@ -1286,13 +1286,13 @@ pipe_write(fp, uio, active_cred, flags, td) > } > > /* > - * Don't return EPIPE if I/O was successful > + * Don't return EPIPE if any byte was written. > + * EINTR and other interrupts are handled by generic I/O layer. > + * Do not pretend that I/O succeeded for obvious user error > + * like EFAULT. > */ > - if ((wpipe->pipe_buffer.cnt == 0) && > - (uio->uio_resid == 0) && > - (error == EPIPE)) { > + if (uio->uio_resid != orig_resid && error == EPIPE) > error = 0; > - } > > if (error == 0) > vfs_timestamp(&wpipe->pipe_mtime); I dislike the patch, if I thought it is right, I would have already submit such a patch. Now let us see why your patch is wore than my version (attached): -current: short write is done, some bytes were sent dofilewrite returns -1, errno is EPIPE dofilewrite sends SIGPIPE application is killed by SIGPIPE -my attached version: short write is done, some bytes were sent dofilewrite return number of bytes sent, errno is zero dofilewrite sends SIGPIPE. application is killed by SIGPIPE -you version: short write is done, some bytes were sent. dofilewrite returns number of bytes sent, errno is zero. dofilewrite does not send SIGPIPE signal application is not killed application does not check return value from write() application thinks it is successful, application does other things, application might begin a bank account transaction. ... application never comes back... my patch is more compatible with -current. if application does not setup a signal handler for SIGPIPE, when short write happens, it is killed, it is same as -current. if the application set up a signal handler for the signal, it always should check the return value from write(), this is how traditional code works. in your patch, short write does not kill application, you can not assume that the application will request a next write() on the pipe, and you hope the second write to kill the application, but there is always exception, the next write does not happen, application works on other things. This is too incompatible with -current.