From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] GnuTLS support on Woe32 Date: Sun, 06 Mar 2011 18:58:46 +0200 Message-ID: <83oc5ogp89.fsf@gnu.org> References: <87ipvwl1nx.wl%claudio.bley@gmail.com> Reply-To: Eli Zaretskii NNTP-Posting-Host: lo.gmane.org X-Trace: dough.gmane.org 1299431004 26587 80.91.229.12 (6 Mar 2011 17:03:24 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Sun, 6 Mar 2011 17:03:24 +0000 (UTC) Cc: emacs-devel@gnu.org To: claudio.bley@gmail.com (Claudio Bley) Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Mar 06 18:03:20 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1PwHMS-0002PC-Hv for ged-emacs-devel@m.gmane.org; Sun, 06 Mar 2011 18:03:20 +0100 Original-Received: from localhost ([127.0.0.1]:42036 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PwHMP-00071H-QT for ged-emacs-devel@m.gmane.org; Sun, 06 Mar 2011 12:03:13 -0500 Original-Received: from [140.186.70.92] (port=53061 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PwHK4-0006HB-DY for emacs-devel@gnu.org; Sun, 06 Mar 2011 12:00:51 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PwHK3-0002eN-1h for emacs-devel@gnu.org; Sun, 06 Mar 2011 12:00:48 -0500 Original-Received: from mtaout22.012.net.il ([80.179.55.172]:47613) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PwHK2-0002dc-Mw for emacs-devel@gnu.org; Sun, 06 Mar 2011 12:00:46 -0500 Original-Received: from conversion-daemon.a-mtaout22.012.net.il by a-mtaout22.012.net.il (HyperSendmail v2007.08) id <0LHN00100A94S500@a-mtaout22.012.net.il> for emacs-devel@gnu.org; Sun, 06 Mar 2011 19:00:44 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([77.124.58.59]) by a-mtaout22.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0LHN001V8B96BE80@a-mtaout22.012.net.il>; Sun, 06 Mar 2011 19:00:44 +0200 (IST) In-reply-to: <87ipvwl1nx.wl%claudio.bley@gmail.com> X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: Solaris 10 (beta) X-Received-From: 80.179.55.172 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:136818 Archived-At: > From: claudio.bley@gmail.com (Claudio Bley) > Date: Sun, 06 Mar 2011 16:16:34 +0100 > > Please find attached a patch which makes building Emacs with GnuTLS > support on Woe32 possible. Thanks. I have a few initial comments, based on reading through the patch. > --- lisp/gnus/starttls.el 2011-01-25 04:08:28 +0000 > +++ lisp/gnus/starttls.el 2011-03-06 14:57:51 +0000 > @@ -195,37 +195,46 @@ > :type 'regexp > :group 'starttls) > > +(eval-and-compile > + (when (fboundp 'gnutls-boot) (require 'gnutls))) Can you explain why are these fboundp calls needed? > --- lisp/net/gnutls.el 2011-01-25 04:08:28 +0000 > +++ lisp/net/gnutls.el 2011-03-06 14:57:51 +0000 > @@ -78,7 +78,8 @@ > KEYFILES is a list of client keys." > (let* ((type (or type 'gnutls-x509pki)) > (trustfiles (or trustfiles > - '("/etc/ssl/certs/ca-certificates.crt"))) > + (when (file-exists-p "/etc/ssl/certs/ca-certificates.crt") > + '("/etc/ssl/certs/ca-certificates.crt")))) Can a file name that starts with a slash work reliably on Windows? > +2011-03-06 Claudio Bley > + > + * configure.bat: New options --without-gnutls and --lib, new build > + variable USER_LIBS, automatically detect GnuTLS. > + * INSTALL: Add instructions for GnuTLS support. > + * gmake.defs: Prefix USER_LIB's with -l. Why do we need the --lib switch? We don't require it for any other optional libraries. Can we arrange for GnuTLS support configury to work like the other optional libraries? > +* Optional GnuTLS support > + > + To build Emacs with GnuTLS support, make sure that the > + gnutls/gnutls.h header file can be found in the include path and > + link to the appropriate libraries (e.g. gnutls.dll and gcrypt.dll) > + using the --lib option. Is it possible to mention here good sites to look for the GnuTLS header and libraries? > +#ifdef WINDOWSNT > +# include "sys/socket.h" > +# include "systime.h" > + > +/* we need to translate Winsock errors because GnuTLS only checks > + * for EAGAIN or EINTR */ > +static int > +wsaerror_to_errno(int err) > +{ > + switch (err) > + { > + case WSAEWOULDBLOCK: > + return EAGAIN; > + case WSAEINTR: > + return EINTR; > + default: > + return err; > + } > +} Why is this function needed? Can you extend w32.c:set_errno instead (if it doesn't already support all the values of WSA* errors that you need)? > +static ssize_t > +emacs_gnutls_pull(gnutls_transport_ptr_t p, void* buf, size_t sz) Can we move the Windows-specific functions to w32.c, and only call them from gnutls.c? I think we want to keep the Windows-related code outside w32*.c to the bare minimum. > + /* On Windows we cannot transfer socket handles between > + * different runtime libraries. > + * > + * We must handle reading / writing ourselves. > + */ This is not the Emacs style of comments. > - ret = gnutls_handshake (state); > + do > + { > + ret = gnutls_handshake (state); > + emacs_gnutls_handle_error (state, ret); > + } > + while (ret < 0 && gnutls_error_is_fatal (ret) == 0); This change is not Windows-specific. What problem(s) does it solve, and are those problems relevant to platforms other than Windows? > + else > + { > + gnutls_alert_send_appropriate (state, ret); > + } > + return ret; Likewise here. > - return (bytes_written ? bytes_written : -1); > + { > + emacs_gnutls_handle_error (state, rtnval); > + > + return (bytes_written ? bytes_written : -1); > + } And here. Why do you introduce emacs_gnutls_handle_error? > --- src/process.c 2011-02-18 17:37:30 +0000 > +++ src/process.c 2011-03-06 14:57:51 +0000 > @@ -4785,6 +4785,21 @@ > &Available, > (check_write ? &Writeok : (SELECT_TYPE *)0), > (SELECT_TYPE *)0, &timeout); > + > +#ifdef HAVE_GNUTLS > + /* > + * GnuTLS buffers data internally. In lowat mode it leaves some data > + * in the TCP buffers so that select works, but with custom pull/push > + * functions we need to check if some data is available in the buffers > + * manually. > + */ > + if (nfds == 0 && wait_proc && wait_proc->gnutls_p > + && gnutls_record_check_pending(wait_proc->gnutls_state) > 0) > + { > + FD_SET (wait_proc->infd, &Available); > + nfds = 1; > + } > +#endif Is this for Windows only? If so, please mention that in a comment. If not, what problems does it solve on other platforms? Last, but not least: I don't see your name on file with the FSF copyright assignments. A contribution of this size will require you to sign legal papers. Thanks again for working on this.