emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] GnuTLS support on Woe32


From: Stefan Monnier
Subject: Re: [PATCH] GnuTLS support on Woe32
Date: Tue, 22 Mar 2011 01:40:06 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux)

> +  hostname = SSDATA (Fsymbol_value (intern_c_string ("gnutls-hostname")));

C is not Lisp, it does not perform dynamic type checks for you, you have
to do them by hand: the above code will lead to crashes if someone sets
gnutls-hostname to something else than a string, so you need to
CHECK_STRING or something like that.

Also further down you define Qgnutls_hostname but never use it, but here
would be a good place to use it (otherwise, don't define it).
Finally, if you want to avoid Fsymbol_value, you can use DEFVAR_LISP to
define Vgnutls_hostname so you can then just do SSDATA (Vgnutls_hostname).

> +  if (peer_verification & GNUTLS_CERT_INVALID)
> +  {
> +    message ("%s certificate could not be verified.", 
> +             hostname);
> +  }

You do not need the braces if there's only one instruction in the block.

> +#ifdef HAVE_GNUTLS
> +          /* GnuTLS buffers data internally. In lowat mode it leaves some 
> data

Shouldn't that be "Iowait"?  Also please put 2 spaces after a ".".

> +              && gnutls_record_check_pending(wait_proc->gnutls_state) > 0)
                                              ^^
                                          needs a space

> +              sc = select (fd + 1, &fdset, (SELECT_TYPE *)0, (SELECT_TYPE 
> *)0, &timeout);

That seems to go way past column 80.  Please fold it.

> +                                  /* translate WSAEWOULDBLOCK alias
> +                                     EWOULDBLOCK to EAGAIN for
> +                                     GnuTLS */

The comment above needs to start with a capital letter and end with a ".".

> +extern ssize_t emacs_gnutls_pull(gnutls_transport_ptr_t p,
> +                              void* buf, size_t sz);
> +extern ssize_t emacs_gnutls_push(gnutls_transport_ptr_t p,
> +                              const void* buf, size_t sz);

Again, the above needs spaces before the open paren.

As far as functionality goes, I don't know what this is trying to do nor
why it needs to do it this way, so I can't really judge.  The key
validation code seems to be "very" complex, in the sense that we would
probably want to move some of that complexity to Elisp at some point.


        Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]