Emacs core TLS support

Stefan Monnier monnier at iro.umontreal.ca
Sun Sep 12 12:58:47 CEST 2010


> patch++

Looks good, except for a few coding style conventions:

> +  } while( rtnval==GNUTLS_E_INTERRUPTED || rtnval==GNUTLS_E_AGAIN);
> +  fsync(STDOUT_FILENO);

Place a space *before* the open-paren and around infix operators.

> +    /* means the we will only be called again if the library cannot
> +     * determine which certificate to send
> +     */

Put the comment-close at the end of the previous line.

> +  // message ("gnutls: setting the trustfile");
> +
> +  // if (EQ (type, Qgnutls_x509pki))
> +  // {
> +  //     CHECK_STRING (trustfile);
> +
> +  //     x509_cred = XPROCESS (proc)->x509_cred;
> +  //     puts("Setting certificate");
> +  //     puts(XSTRING (trustfile)->data);
> +  //     ret = gnutls_certificate_set_x509_trust_file (x509_cred,
> +  //                                                   XSTRING (trustfile)->data,
> +  //                                                   GNUTLS_X509_FMT_PEM);
> +  // }
> +
> +  // if (ret != GNUTLS_E_SUCCESS)
> +  //     return gnutls_make_error (ret);

We use /*..*/ comments, or "#if 0 ... #endif".

> +       doc: /* Terminate current GNU TLS connection for PROCESS.
> +The connection should have been initiated using gnutls_handshake().

This should mention `gnutls-handshake' rather than gnutls_handshake().

BTW, for functions whose are meant to be "internal" (e.g. only expected
to be used via a wrapper in gnutls.el) you can use a "gnutls--" prefix.
This is not a widely used convention in Elisp, but some packages try to
use it.

> +#define GNUTLS_STAGE_EMPTY 0
> +#define GNUTLS_STAGE_CRED_ALLOC 1
> +#define GNUTLS_STAGE_FILES 2
> +#define GNUTLS_STAGE_INIT 3
> +#define GNUTLS_STAGE_PRIORITY 4
> +#define GNUTLS_STAGE_CRED_SET 5

Please use an enum (and use it for the type of the gnutls_initstage
field, of course).

> +#define GNUTLS_STAGE_HANDSHAKE_CANDO 5

Why is that the same value as GNUTLS_STAGE_CRED_SET?

> +#define GNUTLS_STAGE_HANDSHAKE_DONE 6

> +#define GNUTLS_PROCESS_USABLE(proc) ( GNUTLS_INITSTAGE(proc) >= GNUTLS_STAGE_READY )

No need for spaces after the open and before the close paren.

> +#ifdef HAVE_GNUTLS
> +/* Defined in gnutls.c */
> +extern void syms_of_gnutls (void);
> +#endif

Why here rather than in gnutls.h?

Also gnutls.c and gnutls.h need a GPL notice at the beginning.
See other files for the usual boilerplate.

> +  /* AKA GNUTLS_INITSTAGE(proc) */

Please finish your comments with a full-stop (and follow it by 2 spaces).

> +	nbytes = emacs_gnutls_read (channel, XPROCESS (proc)->gnutls_state, chars + carryover + 1, readmax - 1);

Don't overflow the 80th column.


        Stefan




More information about the Gnutls-devel mailing list