unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Ted Zlatanov <tzz@lifelogs.com>
Cc: emacs-devel@gnu.org
Subject: Re: libnettle/libhogweed WIP
Date: Sat, 03 Jun 2017 10:23:39 +0300	[thread overview]
Message-ID: <83tw3xbklg.fsf@gnu.org> (raw)
In-Reply-To: <87o9u8q4a5.fsf@lifelogs.com> (message from Ted Zlatanov on Wed,  31 May 2017 14:17:54 -0400)

> From: Ted Zlatanov <tzz@lifelogs.com>
> 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.




  reply	other threads:[~2017-06-03  7:23 UTC|newest]

Thread overview: 128+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-20 10:00 How to ship native modules? Elias Mårtenson
2017-02-20 15:27 ` Eli Zaretskii
2017-02-20 16:01   ` Elias Mårtenson
2017-02-20 16:30     ` Eli Zaretskii
2017-02-21  2:48       ` Elias Mårtenson
2017-02-21  3:41         ` Eli Zaretskii
2017-02-21  4:13           ` Elias Mårtenson
2017-02-21 16:48             ` Eli Zaretskii
2017-02-21 20:06               ` John Wiegley
2017-02-21 14:44       ` Stefan Monnier
     [not found]         ` <CADtN0WLjNcFRLCsJNZX+XfqOcq+veTaoGkwHQCV9bjvuQoEORA@mail.gmail.com>
2017-02-21 15:48           ` Elias Mårtenson
2017-02-21 17:14             ` Stefan Monnier
2017-02-21 16:59         ` Eli Zaretskii
2017-03-02 14:59   ` request to reconsider libnettle/libhogweed (was: How to ship native modules?) Ted Zlatanov
2017-03-02 15:19     ` request to reconsider libnettle/libhogweed Stefan Monnier
2017-03-02 15:55     ` request to reconsider libnettle/libhogweed (was: How to ship native modules?) Eli Zaretskii
2017-03-15 21:19       ` libnettle/libhogweed WIP (was: request to reconsider libnettle/libhogweed) Ted Zlatanov
2017-03-16 15:28         ` Eli Zaretskii
2017-03-17 22:46           ` libnettle/libhogweed WIP Ted Zlatanov
2017-03-18  8:12             ` Eli Zaretskii
2017-03-20 18:45           ` Ted Zlatanov
2017-04-11 20:05           ` Ted Zlatanov
2017-04-14 20:48             ` Ted Zlatanov
2017-04-15  9:32               ` Eli Zaretskii
2017-04-15 14:27                 ` Ted Zlatanov
2017-04-15 14:55                   ` Eli Zaretskii
2017-04-16  2:39                     ` Ted Zlatanov
2017-04-16  6:25                       ` Eli Zaretskii
2017-04-16  6:51                       ` Eli Zaretskii
2017-04-17 16:23                         ` Ted Zlatanov
2017-04-17 16:34                           ` Eli Zaretskii
2017-04-17 16:55                             ` Ted Zlatanov
2017-04-17 17:11                               ` Eli Zaretskii
2017-04-17 17:34                                 ` Ted Zlatanov
2017-04-17 17:46                                   ` Ted Zlatanov
2017-04-17 18:11                                   ` Eli Zaretskii
2017-04-17 20:50                               ` Ted Zlatanov
2017-04-17 21:19                                 ` Noam Postavsky
2017-04-17 23:29                                   ` Ted Zlatanov
2017-04-19  2:08                                     ` Ted Zlatanov
2017-04-19  2:42                                       ` Noam Postavsky
2017-04-19 15:24                                       ` Davis Herring
2017-04-19 15:45                                       ` Eli Zaretskii
2017-04-20 17:24                                         ` Ted Zlatanov
2017-04-20 19:38                                           ` Eli Zaretskii
2017-04-20 20:24                                             ` Ted Zlatanov
2017-04-20 20:42                                               ` Lars Ingebrigtsen
2017-04-20 21:54                                                 ` Ted Zlatanov
2017-04-21  6:21                                                   ` Eli Zaretskii
2017-04-21 18:45                                                   ` Lars Ingebrigtsen
2017-04-21 19:15                                                     ` Eli Zaretskii
2017-04-21  6:14                                               ` Eli Zaretskii
2017-05-15 21:55                                                 ` Ted Zlatanov
2017-05-16 22:19                                                   ` Ted Zlatanov
2017-05-17 16:22                                                   ` Eli Zaretskii
2017-05-17 20:05                                                     ` Ted Zlatanov
2017-05-31 18:17                                                       ` Ted Zlatanov
2017-06-03  7:23                                                         ` Eli Zaretskii [this message]
2017-06-03  9:00                                                           ` Andreas Schwab
2017-06-03 10:01                                                             ` Eli Zaretskii
2017-06-03 10:09                                                               ` Andreas Schwab
2017-06-03 10:47                                                                 ` Eli Zaretskii
2017-06-27 22:58                                                           ` Ted Zlatanov
2017-06-28 16:54                                                             ` Eli Zaretskii
2017-06-28 19:44                                                               ` Ted Zlatanov
2017-07-13 18:35                                                                 ` Ted Zlatanov
2017-07-14 15:10                                                                   ` Ted Zlatanov
2017-07-14 19:04                                                                     ` Eli Zaretskii
2017-07-14 19:43                                                                       ` Ted Zlatanov
2017-07-14 20:04                                                                         ` Eli Zaretskii
2017-07-15 18:30                                                                           ` Ted Zlatanov
2017-07-15  9:15                                                                         ` Eli Zaretskii
2017-07-15 18:40                                                                           ` Ted Zlatanov
2017-07-15 19:12                                                                             ` Eli Zaretskii
2017-07-22  9:10                                                                               ` Eli Zaretskii
2017-07-26  6:58                                                                                 ` Ted Zlatanov
2017-07-26 14:52                                                                                   ` Eli Zaretskii
2017-07-26 15:34                                                                                     ` Ted Zlatanov
2017-07-26 15:49                                                                                       ` Eli Zaretskii
2017-07-26 16:08                                                                                         ` Ted Zlatanov
2017-07-26 18:51                                                                                           ` Eli Zaretskii
2017-07-26 20:48                                                                                             ` Ted Zlatanov
2017-07-27  0:19                                                                                   ` Paul Eggert
2017-07-27  2:34                                                                                     ` Eli Zaretskii
2017-07-27  4:36                                                                                       ` Paul Eggert
2017-07-27 15:56                                                                                         ` Ted Zlatanov
2017-08-03 19:52                                                                                           ` Ted Zlatanov
2017-08-03  8:02                                                                                         ` Paul Eggert
2017-08-03 16:49                                                                                           ` Eli Zaretskii
2017-04-18 17:44                                 ` Ted Zlatanov
2017-04-19 12:22                               ` Stefan Monnier
2017-04-19 13:38                                 ` Ted Zlatanov
2017-04-19 14:16                                 ` Lars Ingebrigtsen
2017-04-19 14:48                                   ` Stefan Monnier
2017-04-19 14:41                                 ` Eli Zaretskii
2017-04-19 14:54                                   ` Stefan Monnier
2017-04-19 15:31                                     ` Eli Zaretskii
2017-04-19 15:48                                   ` Ted Zlatanov
2017-04-19 16:49                                     ` Lars Ingebrigtsen
2017-04-19 17:24                                       ` Eli Zaretskii
2017-04-19 19:53                                         ` Stefan Monnier
2017-04-20  2:30                                           ` Eli Zaretskii
2017-04-20  3:36                                             ` Stefan Monnier
2017-04-20 15:46                                               ` Eli Zaretskii
2017-04-20 15:59                                                 ` Lars Ingebrigtsen
2017-04-20 16:24                                                   ` Eli Zaretskii
2017-04-20 17:25                                                     ` Stefan Monnier
2017-04-20 19:40                                                       ` Lars Ingebrigtsen
2017-04-20 20:31                                                         ` Eli Zaretskii
2017-04-20 19:58                                                       ` Eli Zaretskii
2017-04-20 20:36                                                         ` Eli Zaretskii
2017-04-20 17:14                                                 ` Stefan Monnier
2017-04-20 19:29                                                   ` Eli Zaretskii
2017-04-19 19:49                                       ` Stefan Monnier
2017-04-17 16:00                       ` rename STRING_SET_CHARS to STRING_SET_SIZE (was: libnettle/libhogweed WIP) Ted Zlatanov
2017-04-17 16:24                         ` rename STRING_SET_CHARS to STRING_SET_SIZE Eli Zaretskii
2017-04-17 16:29                         ` Stefan Monnier
2017-04-17 16:34                           ` Ted Zlatanov
2017-04-16  3:37                     ` libnettle/libhogweed WIP Stefan Monnier
2017-04-16  6:19                       ` Eli Zaretskii
2017-04-16 13:20                         ` Stefan Monnier
2017-04-16  7:47               ` Toon Claes
2017-03-02 17:58     ` request to reconsider libnettle/libhogweed Paul Eggert
2017-03-02 18:33       ` Ted Zlatanov
2017-02-20 15:33 ` How to ship native modules? Aurélien Aptel
2017-02-21  4:50 ` Andreas Politz
2017-02-21  5:12   ` Elias Mårtenson
2017-02-21  5:23     ` Andreas Politz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83tw3xbklg.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    --cc=tzz@lifelogs.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).