Re: GnuTLS and zeroing keys in Emacs

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]

Ted Zlatanov wrote:

> PE> -      Lisp_Object cp = listn (CONSTYPE_HEAP, 15,
> PE> -                              /* A symbol representing the cipher */
> PE> -                              intern (gnutls_cipher_get_name (gca)),
> 
> You removed the comments from these lists in the C code in several
> places, but I do think they are useful to a reader. Can we keep them?

I restored those particular comments by installing the attached patch. That 
being said, I found the comments to have more cost (in terms of clutter) than 
benefit (in terms of useful information conveyed), and similarly for some nearby 
comments that I also removed. For example, anyone who reads the Emacs C code 
should know that "intern (gnutls_cipher_get_name (gca))" returns a symbol 
representing the cipher name, and the nearby comment "/* A symbol representing 
the GnuTLS cipher. */" just gets in the way; it has the feel of "n++; /* 
Increment N. */".

Admittedly this is a judgment call.

> PE> -Returns nil on error.
> PE> +Return nil on error.
> 
> Is this right? It seems it's talking from the POV of the function.

Doc strings for functions should use the imperative form, as it is typically 
shorter and less likely to be misunderstood. For example the doc string for the 
'length' function begins "Return the length of ...".

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-src-gnutls.c-Restore-some-comments.patch --]
[-- Type: text/x-patch; name="0001-src-gnutls.c-Restore-some-comments.patch", Size: 1918 bytes --]

From 8fdadf52567de8dc5979e9b453b16c429f76bd87 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 16 Jul 2017 17:27:03 -0700
Subject: [PATCH] * src/gnutls.c: Restore some comments.

---
 src/gnutls.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/gnutls.c b/src/gnutls.c
index 7d19f90..bcccd7f 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -1857,7 +1857,10 @@ The alist key is the cipher name. */)
   for (ptrdiff_t pos = 0; gciphers[pos] != GNUTLS_CIPHER_NULL; pos++)
     {
       gnutls_cipher_algorithm_t gca = gciphers[pos];
+
+      /* A symbol representing the GnuTLS cipher.  */
       Lisp_Object cipher_symbol = intern (gnutls_cipher_get_name (gca));
+
       ptrdiff_t cipher_tag_size = gnutls_cipher_get_tag_size (gca);
 
       Lisp_Object cp
@@ -2200,9 +2203,10 @@ name. */)
     {
       const gnutls_mac_algorithm_t gma = macs[pos];
 
-      const char *name = gnutls_mac_get_name (gma);
+      /* A symbol representing the GnuTLS MAC algorithm.  */
+      Lisp_Object gma_symbol = intern (gnutls_mac_get_name (gma));
 
-      Lisp_Object mp = listn (CONSTYPE_HEAP, 11, intern (name),
+      Lisp_Object mp = listn (CONSTYPE_HEAP, 11, gma_symbol,
 			      QCmac_algorithm_id, make_number (gma),
 			      QCtype, Qgnutls_type_mac_algorithm,
 
@@ -2236,9 +2240,10 @@ method name. */)
     {
       const gnutls_digest_algorithm_t gda = digests[pos];
 
-      const char *name = gnutls_digest_get_name (gda);
+      /* A symbol representing the GnuTLS digest algorithm.  */
+      Lisp_Object gda_symbol = intern (gnutls_digest_get_name (gda));
 
-      Lisp_Object mp = listn (CONSTYPE_HEAP, 7, intern (name),
+      Lisp_Object mp = listn (CONSTYPE_HEAP, 7, gda_symbol,
 			      QCdigest_algorithm_id, make_number (gda),
 			      QCtype, Qgnutls_type_digest_algorithm,
 
-- 
2.7.4