From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Newsgroups: gmane.lisp.guile.devel Subject: Re: [PATCH] Bindings for =?utf-8?B?4oCYc2VuZGZpbGXigJk=?= Date: Thu, 21 Mar 2013 10:40:47 +0100 Message-ID: <87obed2iqo.fsf@gnu.org> References: <87ip4liufs.fsf@gnu.org> <878v5hbblk.fsf@tines.lan> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: ger.gmane.org 1363858872 16947 80.91.229.3 (21 Mar 2013 09:41:12 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 21 Mar 2013 09:41:12 +0000 (UTC) Cc: guile-devel@gnu.org To: Mark H Weaver Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Thu Mar 21 10:41:38 2013 Return-path: Envelope-to: guile-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1UIc04-0000iw-Fn for guile-devel@m.gmane.org; Thu, 21 Mar 2013 10:41:32 +0100 Original-Received: from localhost ([::1]:46407 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIbzh-0007lW-7d for guile-devel@m.gmane.org; Thu, 21 Mar 2013 05:41:09 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:40671) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIbza-0007kf-T7 for guile-devel@gnu.org; Thu, 21 Mar 2013 05:41:07 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UIbzW-0000gh-5w for guile-devel@gnu.org; Thu, 21 Mar 2013 05:41:02 -0400 Original-Received: from [2a01:e0b:1:123:ca0a:a9ff:fe03:271e] (port=37821 helo=xanadu.aquilenet.fr) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIbzV-0000a7-Ra for guile-devel@gnu.org; Thu, 21 Mar 2013 05:40:58 -0400 Original-Received: from localhost (localhost [127.0.0.1]) by xanadu.aquilenet.fr (Postfix) with ESMTP id 3CE0C9637; Thu, 21 Mar 2013 10:40:48 +0100 (CET) Original-Received: from xanadu.aquilenet.fr ([127.0.0.1]) by localhost (xanadu.aquilenet.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dvI15gCo4JAw; Thu, 21 Mar 2013 10:40:48 +0100 (CET) Original-Received: from pluto (nat-eduroam-36-gw-01-bso.bordeaux.inria.fr [194.199.1.36]) by xanadu.aquilenet.fr (Postfix) with ESMTPSA id ED68E7A4A; Thu, 21 Mar 2013 10:40:47 +0100 (CET) X-URL: http://www.fdn.fr/~lcourtes/ X-Revolutionary-Date: 1 Germinal an 221 de la =?utf-8?Q?R=C3=A9volution?= X-PGP-Key-ID: 0xEA52ECF4 X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc X-PGP-Fingerprint: 83C4 F8E5 10A3 3B4C 5BEA D15D 77DD 95E2 EA52 ECF4 X-OS: x86_64-unknown-linux-gnu In-Reply-To: <878v5hbblk.fsf@tines.lan> (Mark H. Weaver's message of "Thu, 21 Mar 2013 00:50:15 -0400") User-Agent: Gnus/5.130005 (Ma Gnus v0.5) Emacs/24.2 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 2a01:e0b:1:123:ca0a:a9ff:fe03:271e X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Original-Sender: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.lisp.guile.devel:15948 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Mark, Mark H Weaver skribis: > ludo@gnu.org (Ludovic Court=C3=A8s) writes: >> I plan to commit the patch below, which adds bindings for =E2=80=98sendf= ile=E2=80=99. >> >> Comments? > > Looks great to me, modulo one comment below. Thanks for the quick review! > I especially like the fact that although it can make use of the > non-standard Linux syscall, it works properly on all systems. In > response to suggestions by others that we create a "linux" module: > I'd prefer to follow the good precedent set by Ludovic here. Yeah. (And experience has shown that POSIX largely follows GNU/Linux nowadays.) >> + size_t c_count; >> + off_t c_offset; >> + ssize_t result; >> + int in_fd, out_fd; >> + >> + VALIDATE_FD_OR_PORT (out_fd, out, 1); >> + VALIDATE_FD_OR_PORT (in_fd, in, 2); >> + c_count =3D scm_to_size_t (count); > > Since the code below will behave badly if 'c_count' does not fit in an > 'ssize_t', we should validate here that it _does_ fit. Oops, indeed. (Note that sendfile(2) and write(2) have that problem: they take a size_t and return a ssize_t...) There was also the other issue of making sure we use the right function, depending on _FILE_OFFSET_BITS & co. Here are the changes compared to the previous patch: --=-=-= Content-Type: text/x-patch Content-Disposition: inline diff --git a/libguile/filesys.c b/libguile/filesys.c index 097b03a..6804db9 100644 --- a/libguile/filesys.c +++ b/libguile/filesys.c @@ -102,6 +102,10 @@ # include #endif +/* Glibc's `sendfile' function. */ +#define sendfile_or_sendfile64 \ + CHOOSE_LARGEFILE (sendfile, sendfile64) + #include #include @@ -1123,7 +1127,7 @@ SCM_DEFINE (scm_sendfile, "sendfile", 3, 1, 0, } size_t c_count; - off_t c_offset; + scm_t_off c_offset; ssize_t result; int in_fd, out_fd; @@ -1133,20 +1137,20 @@ SCM_DEFINE (scm_sendfile, "sendfile", 3, 1, 0, c_offset = SCM_UNBNDP (offset) ? 0 : scm_to_off_t (offset); #ifdef HAVE_SENDFILE - result = sendfile (out_fd, in_fd, + result = sendfile_or_sendfile64 (out_fd, in_fd, SCM_UNBNDP (offset) ? NULL : &c_offset, c_count); /* Quoting the Linux man page: "In Linux kernels before 2.6.33, out_fd must refer to a socket. Since Linux 2.6.33 it can be any file." - Fall back to read(2) and write(2) such an error happens. */ + Fall back to read(2) and write(2) when such an error occurs. */ if (result < 0 && errno != EINVAL && errno != ENOSYS) SCM_SYSERROR; else if (result < 0) #endif { char buf[8192]; - size_t left; + size_t result, left; if (!SCM_UNBNDP (offset)) { @@ -1173,6 +1177,8 @@ SCM_DEFINE (scm_sendfile, "sendfile", 3, 1, 0, result += obtained; } + + return scm_from_size_t (result); } return scm_from_ssize_t (result); --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable WDYT? Thanks, Ludo=E2=80=99. --=-=-=--