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