From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: libnettle/libhogweed WIP Date: Sat, 03 Jun 2017 10:23:39 +0300 Message-ID: <83tw3xbklg.fsf@gnu.org> References: <8337daggnj.fsf@gnu.org> <87d1cdwxt6.fsf@lifelogs.com> <83tw5pg1q3.fsf@gnu.org> <87zifhulc2.fsf@lifelogs.com> <83h91og80k.fsf@gnu.org> <87pogbuhoe.fsf@lifelogs.com> <834lxndmd9.fsf@gnu.org> <87efwrug6z.fsf@lifelogs.com> <87r30qu5av.fsf@lifelogs.com> <874lxmtxyy.fsf@lifelogs.com> <87r30prvwt.fsf@lifelogs.com> <8337d4csez.fsf@gnu.org> <87r30nq9el.fsf@lifelogs.com> <83inlyc1k2.fsf@gnu.org> <87inlyrfni.fsf@lifelogs.com> <837f2eb845.fsf@gnu.org> <87ziedpyy1.fsf@lifelogs.com> <83d1b75u8a.fsf@gnu.org> <87r2znntaq.fsf@lifelogs.com> <87o9u8q4a5.fsf@lifelogs.com> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org X-Trace: blaine.gmane.org 1496474649 841 195.159.176.226 (3 Jun 2017 07:24:09 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 3 Jun 2017 07:24:09 +0000 (UTC) Cc: emacs-devel@gnu.org To: Ted Zlatanov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Jun 03 09:24:02 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dH3Pm-00086V-ED for ged-emacs-devel@m.gmane.org; Sat, 03 Jun 2017 09:24:02 +0200 Original-Received: from localhost ([::1]:52805 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dH3Po-0007Uw-8b for ged-emacs-devel@m.gmane.org; Sat, 03 Jun 2017 03:24:04 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:39533) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dH3Pi-0007Ur-1C for emacs-devel@gnu.org; Sat, 03 Jun 2017 03:23:59 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dH3Pe-0007JD-RL for emacs-devel@gnu.org; Sat, 03 Jun 2017 03:23:58 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:41444) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dH3Pe-0007Iy-Ke; Sat, 03 Jun 2017 03:23:54 -0400 Original-Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:4473 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1dH3Pd-0006fs-Tm; Sat, 03 Jun 2017 03:23:54 -0400 In-reply-to: <87o9u8q4a5.fsf@lifelogs.com> (message from Ted Zlatanov on Wed, 31 May 2017 14:17:54 -0400) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:215422 Archived-At: > From: Ted Zlatanov > Date: Wed, 31 May 2017 14:17:54 -0400 > > I'd love to merge this branch, if there are no objections or comments on > the two items above or otherwise. It's been sitting for a while. I think it's okay to merge this, after fixing the following minor issues: There are a few TODOs in the documentation and in the code. The one in the docs should be either removed or moved to a comment so that it doesn't appear in the manual. Those in the code should be reviewed, and in each case please decide whether the TODO will be handled any time soon, or maybe should be simply deleted. This fragment from the docs: +The functions described here accept argument lists of the +form @code{(BUFFER-OR-STRING START END CODING-SYSTEM NOERROR)}, +where BUFFER-OR-STRING ... + +can be a string or a buffer or a list in the format +@var{(BUFFER-OR-STRING START END CODING-SYSTEM NOERROR)} where only +the first element is required. should have @w{..} around the long form, to prevent it from being broken between 2 lines. I also don't understand the ellipsis which breaks the sentence in two -- I think it should simply be deleted. As for using @var, you should do it for every element individually, and the elements should not be up-cased. Likewise in other uses of @var. In several cases in the docs (manual and doc strings) and in error messages you have only one space between sentences; please use two everywhere. This text: +@item (@code{iv-auto} @var{length}) +This will generate an IV of the specified length using the GnuTLS +GNUTLS_RND_NONCE generator. uses "IV" and "iv-auto" without explaining what that is. Other parts have the same problem. In this fragment: +The @var{input} can be specified as a buffer or string or in other +ways (@xref{Format of GnuTLS Cryptography Inputs}). please use @pxref, as @xref is not appropriate for parenthesized reference. There are several instances of this in the docs changes. Please make sure that "nil" is always in @code in the docs. + Lisp_Object object = Fnth (make_number (0), spec); + Lisp_Object start = Fnth (make_number (1), spec); + Lisp_Object end = Fnth (make_number (2), spec); + Lisp_Object coding_system = Fnth (make_number (3), spec); + Lisp_Object noerror = Fnth (make_number (4), spec); Isn't it simpler to use XCAR and XCDR here? You are extracting all the elements of a list in strict order, so Fnth is an overkill, and will be slower. + if (! INTEGERP (start)) + { + error ("Without a length, 'iv-auto can't be used. See manual."); + object = Qnil; I think the quoting of iv-auto here is incorrect. Likewise in other similar error messages + void *(*hash_func) (const char *, size_t, void *); The leftmost '*' is unnecessary, I think. + Lisp_Object cp = listn (CONSTYPE_HEAP, 15, + // A symbol representing the cipher + intern (gnutls_cipher_get_name (gca)), + // The internally meaningful cipher ID I think we still prefer the /* .. */ style of comments. + if (ret < GNUTLS_E_SUCCESS) + { + const char* str = gnutls_strerror (ret); + if (!str) + str = "unknown"; + error ("GnuTLS AEAD cipher %s/%s initialization failed: %s", + gnutls_cipher_get_name (gca), desc, str); + return Qnil; + } You don't need a 'return' after calling 'error' (here and elsewhere), as the latter doesn't return. Some of the lines in doc strings you wrote are too long, please make sure they are no longer than 76 characters, and in any case fit on a single 80-column line. Also, the first line of a doc string should be a complete sentence. This one, for example, isn't: +DEFUN ("gnutls-digests", Fgnutls_digests, Sgnutls_digests, 0, 0, 0, + doc: /* Return alist of GnuTLS digest-algorithm method +descriptions as plists. Use the value of the alist (extract it with +`alist-get' for instance) with `gnutls-hash-digest'. The alist key is +the digest-algorithm method name. */) + if (STRINGP (hash_method)) + { + hash_method = intern (SSDATA (hash_method)); + } No need for braces when there's only 1 line in the block. Thanks again for working on this.