From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.comp.encryption.gpg.gnutls.devel,gmane.emacs.devel Subject: Re: Emacs core TLS support Date: Mon, 06 Sep 2010 23:00:51 +0200 Message-ID: References: <878wc1vfh3.fsf@lifelogs.com> <87r5ptpnz2.fsf@stupidchicken.com> <871vhsvkut.fsf@lifelogs.com> <87d41csktn.fsf@lifelogs.com> <87k4v0n0m8.fsf@lifelogs.com> <87wrrvfnc4.fsf@lifelogs.com> <87r5i2d00q.fsf@lifelogs.com> <87zkwqijye.fsf@stupidchicken.com> <878w4actmg.fsf@lifelogs.com> <877hju123h.fsf@stupidchicken.com> <8762yklrdk.fsf@lifelogs.com> <87wrqzhrjv.fsf@lifelogs.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1283806873 5514 80.91.229.12 (6 Sep 2010 21:01:13 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 6 Sep 2010 21:01:13 +0000 (UTC) Cc: gnutls-devel@gnu.org, emacs-devel@gnu.org To: Ted Zlatanov Original-X-From: gnutls-devel-bounces+pgp-gnutls-dev=m.gmane.org@gnu.org Mon Sep 06 23:01:11 2010 Return-path: Envelope-to: pgp-gnutls-dev@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 1OsioR-0007ET-D1 for pgp-gnutls-dev@m.gmane.org; Mon, 06 Sep 2010 23:01:11 +0200 Original-Received: from localhost ([127.0.0.1]:45116 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OsioQ-0002Tq-TJ for pgp-gnutls-dev@m.gmane.org; Mon, 06 Sep 2010 17:01:10 -0400 Original-Received: from [140.186.70.92] (port=51588 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OsioN-0002SZ-Fj for gnutls-devel@gnu.org; Mon, 06 Sep 2010 17:01:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OsioM-0003gX-3L for gnutls-devel@gnu.org; Mon, 06 Sep 2010 17:01:07 -0400 Original-Received: from impaqm1.telefonica.net ([213.4.138.1]:41608) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OsioL-0003gP-OW for gnutls-devel@gnu.org; Mon, 06 Sep 2010 17:01:06 -0400 Original-Received: from IMPmailhost4.adm.correo ([10.20.102.125]) by IMPaqm1.telefonica.net with bizsmtp id 3VPG1f01u2iL0W201Z0sAw; Mon, 06 Sep 2010 23:00:52 +0200 Original-Received: from ceviche.home ([83.61.51.178]) by IMPmailhost4.adm.correo with BIZ IMP id 3Z0r1f00D3qhCuj1kZ0rGk; Mon, 06 Sep 2010 23:00:52 +0200 X-Brightmail-Tracker: AAAAAA== X-TE-authinfo: authemail="monnier$movistar.es" |auth_email="monnier@movistar.es" X-TE-AcuTerraCos: auth_cuTerraCos="cosuitnetc01" Original-Received: by ceviche.home (Postfix, from userid 20848) id 6485E660BB; Mon, 6 Sep 2010 23:00:51 +0200 (CEST) In-Reply-To: <87wrqzhrjv.fsf@lifelogs.com> (Ted Zlatanov's message of "Mon, 06 Sep 2010 09:31:32 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-BeenThere: gnutls-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GnuTLS development discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: gnutls-devel-bounces+pgp-gnutls-dev=m.gmane.org@gnu.org Errors-To: gnutls-devel-bounces+pgp-gnutls-dev=m.gmane.org@gnu.org Xref: news.gmane.org gmane.comp.encryption.gpg.gnutls.devel:4481 gmane.emacs.devel:129722 Archived-At: > Yeah, I'll do that for `gnutls-bye' with a CONT parameter. It's just a > NILP check to handle booleans IIUC (there's no CHECK_BOOLEAN, so it's > either NILP or EQ to Qt). Yes, NILP is the way to test it (anything that's not nil is considered as a way to say "true"). >>> +DEFUN ("gnutls-cred-set", Fgnutls_cred_set, >>> + Sgnutls_cred_set, 2, 2, 0, >>> + doc: /* Enables GNU TLS authentication for PROCESS. >>> +TYPE is an integer indicating the type of the credentials, either >>> +`gnutls-anon', `gnutls-srp' or `gnutls-x509pki'. SM> Again, the above formulation means that the caller should pass those SM> symbols rather than value associated with the corresponding variables. > In this case I think that's the right approach actually so we shouldn't > have defconsts. See new definition, it uses two local Lisp_Objects for > the symbol names. > Where should I allocate those constant Lisp_Objects > globally properly? It's typically declared as global (static or not, depending on whether it's used in other files) and initialized in syms_of_. Look at other syms_of_ to see what it looks like. > And should I default to anonymous? I don't know what that means. >>> +#ifdef HAVE_GNUTLS >>> + /* XXX Store GNU TLS state and auth mechanisms in Lisp_Objects. */ >>> + Lisp_Object gnutls_state; >>> + Lisp_Object x509_cred, x509_callback; >>> + Lisp_Object anon_cred; >>> + Lisp_Object srp_cred; >>> +#endif These extra slots don't actually Lisp objects, so this type is wrong (and it forces you to use casts: any use of cast is a sign of a problem, tho admittedly C often makes problems unavoidable). If you check the "struct Lisp_Process" in process.h you'll see this: /* After this point, there are no Lisp_Objects any more. */ /* alloc.c assumes that `pid' is the first such non-Lisp slot. */ so the slots you add at the end are ignored by the GC (which is what you want in your case, since they're not Lisp objects and hence the GC wouldn't know what to do with them) and can be of any type. So just use the types you need here such that casts aren't needed. > I'd like to take the direction of a more Lisp-y API on top of the GnuTLS > API. So any constants should be limited to the function bodies and I'd > like to stick to symbols (as with gnutls-cred-set in the new patch). BTW, if it makes the code simpler, you can use the following trick: use symbols, but associate an integer to each symbol by way of symbol properties. I.e. something like static Lisp_Object Qgnutls_foo; gnutls_function (Lisp_Object arg1, ...) { arg1 = Fget (arg1, Qgnutls_code); int op = INTEGERP (arg1) ? XINT (arg1) : ....; if (op < n1 || op > n2) error ("Invalid op code"); gnutls_foo (..., op, ...); } syms_of_gnutls () { ... Qgnutls_foo = intern ("gnutls-foo"); ... Fput (Qgnutls_foo, Qgnutls_code, make_number (GNUTLS_FOO)); } It's not super-lightweight, but it can be more convenient than a bunch of (EQ (foo, Qgnutls_bar)) if there are many different possible values. >>> + ret = gnutls_init((gnutls_session_t*)&(XPROCESS(proc)->gnutls_state), AS> Aliasing violation. > Can you explain please? Casts are evil, try real hard to stay away from them (he said, and then added a few casts). AS> IMHO all your functions should return t on success and either some error AS> symbol on failure or even raise an error. > Yes, but I'm not sure which one. Can you recommend? error ("some informative string"); > I don't know enough (the choice of using Lisp_Objects was in the > original patch) to know what to do instead of using Lisp_Objects. Why > not, first of all? The type you declare should correspond to the type of the objects you store there. Always. If you stick to this principle and avoid freeing live objects (and stay within array bounds, and a few more such things) your code will be more portable and won't dump core (hence will be generally easier to debug). Stefan