From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Mark H Weaver Newsgroups: gmane.lisp.guile.devel Subject: Re: [PATCH] Bindings for =?utf-8?B?4oCYc2VuZGZpbGXigJk=?= Date: Thu, 21 Mar 2013 00:50:15 -0400 Message-ID: <878v5hbblk.fsf@tines.lan> References: <87ip4liufs.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Trace: ger.gmane.org 1363841437 32616 80.91.229.3 (21 Mar 2013 04:50:37 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 21 Mar 2013 04:50:37 +0000 (UTC) Cc: guile-devel@gnu.org To: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Thu Mar 21 05:51:03 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 1UIXSv-0000F1-VO for guile-devel@m.gmane.org; Thu, 21 Mar 2013 05:51:02 +0100 Original-Received: from localhost ([::1]:51402 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIXSY-0003L5-JR for guile-devel@m.gmane.org; Thu, 21 Mar 2013 00:50:38 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:59138) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIXSS-0003L0-Hb for guile-devel@gnu.org; Thu, 21 Mar 2013 00:50:34 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UIXSR-0005lU-9y for guile-devel@gnu.org; Thu, 21 Mar 2013 00:50:32 -0400 Original-Received: from world.peace.net ([96.39.62.75]:43713) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIXSR-0005k6-5P; Thu, 21 Mar 2013 00:50:31 -0400 Original-Received: from 209-6-91-212.c3-0.smr-ubr1.sbo-smr.ma.cable.rcn.com ([209.6.91.212] helo=tines.lan) by world.peace.net with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.72) (envelope-from ) id 1UIXSI-0001xJ-Q2; Thu, 21 Mar 2013 00:50:23 -0400 In-Reply-To: <87ip4liufs.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Wed, 20 Mar 2013 23:21:27 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 96.39.62.75 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:15942 Archived-At: Hi Ludovic, ludo@gnu.org (Ludovic Court=C3=A8s) writes: > I plan to commit the patch below, which adds bindings for =E2=80=98sendfi= le=E2=80=99. > > Comments? Looks great to me, modulo one comment below. 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. > diff --git a/libguile/filesys.c b/libguile/filesys.c > index 282ff31..097b03a 100644 > --- a/libguile/filesys.c > +++ b/libguile/filesys.c > @@ -98,6 +98,14 @@ >=20=20 > #define NAMLEN(dirent) strlen ((dirent)->d_name) >=20=20 > +#ifdef HAVE_SYS_SENDFILE_H > +# include > +#endif > + > +#include > +#include I didn't know about Gnulib's 'full-read' and 'full-write'. Nice! We should probably make more use of these throughout the tree. > +SCM_DEFINE (scm_sendfile, "sendfile", 3, 1, 0, > + (SCM out, SCM in, SCM count, SCM offset), > + "Send @var{count} bytes from @var{in} to @var{out}, both of which " > + "are either open file ports or file descriptors. When " > + "@var{offset} is omitted, start reading from @var{in}'s current " > + "position; otherwise, start reading at @var{offset}.") > +#define FUNC_NAME s_scm_sendfile > +{ > +#define VALIDATE_FD_OR_PORT(cvar, svar, pos) \ > + if (scm_is_integer (svar)) \ > + cvar =3D scm_to_int (svar); \ > + else \ > + { \ > + SCM_VALIDATE_OPFPORT (pos, svar); \ > + scm_flush (svar); \ > + cvar =3D SCM_FPORT_FDES (svar); \ > + } > + > + 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. > + c_offset =3D SCM_UNBNDP (offset) ? 0 : scm_to_off_t (offset); > + > +#ifdef HAVE_SENDFILE > + result =3D sendfile (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. */ > + if (result < 0 && errno !=3D EINVAL && errno !=3D ENOSYS) > + SCM_SYSERROR; > + else if (result < 0) > +#endif > + { > + char buf[8192]; > + size_t left; > + > + if (!SCM_UNBNDP (offset)) > + { > + if (SCM_PORTP (in)) > + scm_seek (in, offset, scm_from_int (SEEK_SET)); > + else > + lseek_or_lseek64 (in_fd, c_offset, SEEK_SET); > + } > + > + for (result =3D 0, left =3D c_count; result < c_count; ) If 'c_count's does not fit in a 'ssize_t', then this loop will go forever and 'result' will wrap around to negative numbers and undefined C behavior. > + { > + size_t asked, obtained; > + > + asked =3D SCM_MIN (sizeof buf, left); > + obtained =3D full_read (in_fd, buf, asked); > + if (obtained < asked) > + SCM_SYSERROR; > + > + left -=3D obtained; > + > + obtained =3D full_write (out_fd, buf, asked); > + if (obtained < asked) > + SCM_SYSERROR; > + > + result +=3D obtained; > + } > + } > + > + return scm_from_ssize_t (result); > + > +#undef VALIDATE_FD_OR_PORT > +} > +#undef FUNC_NAME > + > #endif /* HAVE_POSIX */ Other than that, looks beautiful to me :) Thanks! Mark