Re: GNU Emacs-libnettle-libhogweed integration patch v1

[Ted Zlatanov]

On Sun, 06 Oct 2013 19:09:41 +0300 Eli Zaretskii <address@hidden> wrote: 

EZ> For the record: this will have to be changed for MS-Windows, to
EZ> support dynamic loading, like we do, e.g., in decompress.c.

Yup, I figured it would, but I don't know that part of the code well.

>> +DEFUN ("nettle-hmac", Fnettle_hmac, Snettle_hmac, 3, 3, 0,
>> +       doc: /* Hash INPUT string with HASH-METHOD and KEY into a unibyte 
>> string according to HMAC (RFC 2104).

EZ> The first line of the doc string is too long.  (There are others with
EZ> similar problems.)

OK, redone.  One just can't be shortened easily:

Make PBKDF2 of HASH-LENGTH from KEY with HASH-METHOD using ITERATIONS and SALT.

EZ> Instead of xzalloc, then make_unibyte_string, I think you should be
EZ> able to allocate a string first, and then work on its SDATA.  This
EZ> will save you from at least one allocation.

Hmm, I thought about that, but typically Nettle wants to work on pure C
strings and it's nice to avoid corrupting Lisp strings.  Other than
saving the allocation, are there other benefits?

EZ> We tend to use xfree, not free.

OK; fixed.

>> +  sha1_mode = 0 == strcmp (SDATA (hash_method), "sha1");
>> +  sha256_mode = 0 == strcmp (SDATA (hash_method), "sha256");

EZ> Perhaps using Fstring_equal is better here, not sure.

OK with me either way.  I didn't want to use symbols because Nettle uses
strings... but maybe symbols are the right approach anyhow.

>> +      uint8_t *digest = xzalloc (outlength);
EZ>          ^^^^^^^
EZ> Why not 'unsigned char'?

Nettle uses those.

>> +  if (!rsa_keypair_from_der (&key, NULL, 0, SCHARS (public_key), SDATA 
>> (public_key)) &&
>> +      !rsa_keypair_from_sexp (&key, NULL, 0, SCHARS (public_key), SDATA 
>> (public_key)))

EZ> The && should be at the beginning of a line, not at the end.

Oh, I always forget that.  Fixed.

I won't post the revised patch, it's too trivial :) But I could publish
the branch in the Emacs repo if it's OK with you and/or Stefan?

Ted