From owner-freebsd-current@FreeBSD.ORG Wed Dec 21 20:36:42 2011 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 162EA1065678; Wed, 21 Dec 2011 20:36:42 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id DCD6A8FC12; Wed, 21 Dec 2011 20:36:41 +0000 (UTC) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [96.47.65.170]) by cyrus.watson.org (Postfix) with ESMTPSA id 93E8846B09; Wed, 21 Dec 2011 15:36:41 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 26E0EB964; Wed, 21 Dec 2011 15:36:41 -0500 (EST) From: John Baldwin To: Kostik Belousov Date: Wed, 21 Dec 2011 15:34:18 -0500 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p8; KDE/4.5.5; amd64; ; ) References: <201112201649.06265.jhb@freebsd.org> <201112211225.18581.jhb@freebsd.org> <20111221202842.GZ50300@deviant.kiev.zoral.com.ua> In-Reply-To: <20111221202842.GZ50300@deviant.kiev.zoral.com.ua> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201112211534.18997.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Wed, 21 Dec 2011 15:36:41 -0500 (EST) Cc: mdf@freebsd.org, Robert Watson , current@freebsd.org Subject: Re: extattr_set_*() return type X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Dec 2011 20:36:42 -0000 On Wednesday, December 21, 2011 3:28:42 pm Kostik Belousov wrote: > On Wed, Dec 21, 2011 at 12:25:18PM -0500, John Baldwin wrote: > > On Wednesday, December 21, 2011 11:13:10 am Kostik Belousov wrote: > > > On Wed, Dec 21, 2011 at 10:31:11AM -0500, John Baldwin wrote: > > > > On Tuesday, December 20, 2011 5:18:58 pm mdf@freebsd.org wrote: > > > > > On Tue, Dec 20, 2011 at 1:49 PM, John Baldwin wrote: > > > > > > Hmm, if these functions are expected to operate like 'write(2)' and are > > > > > > supposed to return the number of bytes written, shouldn't their return value > > > > > > be 'ssize_t' instead of 'int'? It looks like the system calls themselves > > > > > > already do the right thing in setting td_retval[] (they assign a ssize_t to it > > > > > > and td_retval[0] can hold a ssize_t on all of our current platforms). It > > > > > > would seem that the only change would be to the header and probably > > > > > > syscalls.master. I guess this would require a symver bump to fix though. > > > > > > > > > > An extended attribute larger than 2GB is a programming abuse, though. > > > > > Technically int may not be 32 bits but it is on all supported > > > > > platforms now. > > > > > > > > Today it is an abuse. In the 90's a 64-bit off_t was considered an abuse by > > > > some. :) > > > > > > > > The type should match the documented behavior. On OS X the set operation > > > > doesn't return a size but instead returns a simple success/failure (0 or -1) > > > > for which an int is appropriate. However, the FreeBSD API documents that it > > > > operates like write and consumes the buffer. Note that the size of the > > > > buffer passed to the 'set' and 'get' operations is a size_t, not an int, and > > > > the 'get' operations already return a ssize_t, not an int. > > > > > > Note that read(2)/write(2) do return int. I still have WIP patch to fix > > > this, but after some conversations with Bruce I am not sure it is worth > > > finishing. > > > > The manpages and /usr/include/unistd.h claim they return ssize_t. Is this > > related to the changes to make uio_resid a size_t (I thought that went into > > the tree)? If the problem is that the values read/write return may fall into > > the range of only an int even on 64-bit platforms, that is different from the > > return type which is part of the ABI. > Yes, it is related. The type change for uio was done in advance. > > Take a look at the first statement of sys_read() and sys_write(): > if (uap->nbyte > INT_MAX) > return (EINVAL); > and at the copyinio(), which is used by scatter/gather versions of i/o > syscalls to copy in uiovec: > if (iov->iov_len > INT_MAX - uio->uio_resid) { > free(uio, M_IOV); > return (EINVAL); Fair enough, but that is more of an implementation detail. The API/ABI is still correct and uses ssize_t. :) -- John Baldwin