From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] GnuTLS support on Woe32 Date: Tue, 22 Mar 2011 01:40:06 -0400 Message-ID: References: <87ipvwl1nx.wl%claudio.bley@gmail.com> <83oc5ogp89.fsf@gnu.org> <87ipvuwslp.wl%claudio.bley@gmail.com> <87hbbc0zi6.wl%claudio.bley@gmail.com> <83oc5gsdwc.fsf@gnu.org> <87ei6bunxz.wl%claudio.bley@gmail.com> <83tyf6rhgn.fsf@gnu.org> <84zkoy6tah.wl%claudio.bley@gmail.com> <87hbb5a4xq.fsf@lifelogs.com> <84ipvkx1da.wl%claudio.bley@gmail.com> <87d3ls7n3b.fsf@lifelogs.com> <87vczdwjuk.wl%claudio.bley@gmail.com> <87k4fr3koi.fsf@lifelogs.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1300772424 17233 80.91.229.12 (22 Mar 2011 05:40:24 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Tue, 22 Mar 2011 05:40:24 +0000 (UTC) Cc: emacs-devel@gnu.org To: Ted Zlatanov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Mar 22 06:40: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 1Q1uKK-0003ea-2t for ged-emacs-devel@m.gmane.org; Tue, 22 Mar 2011 06:40:20 +0100 Original-Received: from localhost ([127.0.0.1]:45645 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q1uKJ-00039a-En for ged-emacs-devel@m.gmane.org; Tue, 22 Mar 2011 01:40:19 -0400 Original-Received: from [140.186.70.92] (port=56150 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q1uK9-00037n-86 for emacs-devel@gnu.org; Tue, 22 Mar 2011 01:40:10 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q1uK8-0008Rg-4q for emacs-devel@gnu.org; Tue, 22 Mar 2011 01:40:09 -0400 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:63094 helo=ironport2-out.pppoe.ca) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q1uK8-0008RZ-18 for emacs-devel@gnu.org; Tue, 22 Mar 2011 01:40:08 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AvsEANLSh01Ld/X5/2dsb2JhbAClQ3jCYIVjBJVp X-IronPort-AV: E=Sophos;i="4.63,224,1299474000"; d="scan'208";a="97512656" Original-Received: from 75-119-245-249.dsl.teksavvy.com (HELO ceviche.home) ([75.119.245.249]) by ironport2-out.pppoe.ca with ESMTP/TLS/ADH-AES256-SHA; 22 Mar 2011 01:40:06 -0400 Original-Received: by ceviche.home (Postfix, from userid 20848) id B1B24660AF; Tue, 22 Mar 2011 01:40:06 -0400 (EDT) In-Reply-To: <87k4fr3koi.fsf@lifelogs.com> (Ted Zlatanov's message of "Mon, 21 Mar 2011 22:20:45 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 206.248.154.181 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:137511 Archived-At: > + hostname = SSDATA (Fsymbol_value (intern_c_string ("gnutls-hostname"))); C is not Lisp, it does not perform dynamic type checks for you, you have to do them by hand: the above code will lead to crashes if someone sets gnutls-hostname to something else than a string, so you need to CHECK_STRING or something like that. Also further down you define Qgnutls_hostname but never use it, but here would be a good place to use it (otherwise, don't define it). Finally, if you want to avoid Fsymbol_value, you can use DEFVAR_LISP to define Vgnutls_hostname so you can then just do SSDATA (Vgnutls_hostname). > + if (peer_verification & GNUTLS_CERT_INVALID) > + { > + message ("%s certificate could not be verified.", > + hostname); > + } You do not need the braces if there's only one instruction in the block. > +#ifdef HAVE_GNUTLS > + /* GnuTLS buffers data internally. In lowat mode it leaves some data Shouldn't that be "Iowait"? Also please put 2 spaces after a ".". > + && gnutls_record_check_pending(wait_proc->gnutls_state) > 0) ^^ needs a space > + sc = select (fd + 1, &fdset, (SELECT_TYPE *)0, (SELECT_TYPE *)0, &timeout); That seems to go way past column 80. Please fold it. > + /* translate WSAEWOULDBLOCK alias > + EWOULDBLOCK to EAGAIN for > + GnuTLS */ The comment above needs to start with a capital letter and end with a ".". > +extern ssize_t emacs_gnutls_pull(gnutls_transport_ptr_t p, > + void* buf, size_t sz); > +extern ssize_t emacs_gnutls_push(gnutls_transport_ptr_t p, > + const void* buf, size_t sz); Again, the above needs spaces before the open paren. As far as functionality goes, I don't know what this is trying to do nor why it needs to do it this way, so I can't really judge. The key validation code seems to be "very" complex, in the sense that we would probably want to move some of that complexity to Elisp at some point. Stefan