From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [Patch v3 2/2] tools/libxc: Implement writev_exact() in the same style as write_exact() Date: Fri, 18 Jul 2014 13:32:08 +0100 Message-ID: <53C913C8.6070502@citrix.com> References: <1405521126-17035-1-git-send-email-andrew.cooper3@citrix.com> <1405521126-17035-2-git-send-email-andrew.cooper3@citrix.com> <53C874DF.2040903@cn.fujitsu.com> <53C8E6DD.4080207@citrix.com> <1405677220.13883.10.camel@kazak.uk.xensource.com> <53C8F160.7040005@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53C8F160.7040005@cn.fujitsu.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wen Congyang , Ian Campbell Cc: Ian Jackson , Xen-devel List-Id: xen-devel@lists.xenproject.org On 18/07/14 11:05, Wen Congyang wrote: > At 07/18/2014 05:53 PM, Ian Campbell Wrote: >> On Fri, 2014-07-18 at 10:20 +0100, Andrew Cooper wrote: >>> On 18/07/14 02:14, Wen Congyang wrote: >>>> At 07/16/2014 10:32 PM, Andrew Cooper Wrote: >>>>> This implementation of writev_exact() will cope with an iovcnt greater than >>>>> IOV_MAX because glibc will actually let this work anyway, and it is very >>>>> useful not to have to work about this in the caller of writev_exact(). The >>>>> caller is still required to ensure that the sum of iov_len's doesn't overflow >>>>> a ssize_t. >>>>> >>>>> Signed-off-by: Andrew Cooper >>>>> CC: Ian Campbell >>>>> CC: Ian Jackson >>>>> >>>>> --- >>>>> v3: >>>>> * Re-add adjustment for partial writes. >>>>> * Split min/max adjustment into separate patch. >>>>> >>>>> v2: >>>>> * Remove adjustment for partial writes of a specific iov[] entry. >>>>> --- >>>>> tools/libxc/xc_private.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++ >>>>> tools/libxc/xc_private.h | 2 ++ >>>>> 2 files changed, 62 insertions(+) >>>>> >>>>> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c >>>>> index 1c214dd..0941b06 100644 >>>>> --- a/tools/libxc/xc_private.c >>>>> +++ b/tools/libxc/xc_private.c >>>>> @@ -858,6 +858,66 @@ int write_exact(int fd, const void *data, size_t size) >>>>> return 0; >>>>> } >>>>> >>>>> +int writev_exact(int fd, const struct iovec *iov, int iovcnt) >>>>> +{ >>>>> + struct iovec *local_iov = NULL; >>>>> + int rc = 0, iov_idx = 0, saved_errno = 0; >>>>> + ssize_t len; >>>>> + >>>>> + while ( iov_idx < iovcnt ) >>>>> + { >>>>> + /* Skip over iov[] entries with 0 length. */ >>>>> + while ( iov[iov_idx].iov_len == 0 ) >>>>> + if ( ++iov_idx == iovcnt ) >>>>> + goto out; >>>> set saved_errn to 0 before goto out? >>> Good catch. >> Isn't this a success path? errno is generally undefined on success. > Yes, but we set saved_errno to 0 here: >> + saved_errno = 0; >> + >> + out: >> + free(local_iov); >> + errno = saved_errno; >> + return rc; >> +} > I think there is no need to save errno in this function, because > we return -1 when writev()/malloc() fails. > > Another problem: I am deliberately creating the same (somewhat quirky) error semantics of write_exact(), for consistency reasons. The key point is that EOF is -1 and errno of 0. >> + local_iov = malloc(iovcnt * sizeof(*iov)); >> + if ( !local_iov ) >> + { >> + saved_errno = ENOMEM; >> + goto out; >> + } >> + > rc is not set to -1 before goto out. > > Thanks > Wen Congyang Good point. ~Andrew