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