[Openvpn-devel,v2,2/3] Minor style change to improve code style

Message ID 20200416113930.15192-2-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v2,1/3] Use crypto library functions for const time memcmp when possible | expand

Commit Message

Arne Schwabe April 16, 2020, 1:39 a.m. UTC
These are small manual changes that are done to improve the code
style and also make the result of uncrustify better without mixing
manual changes/automatic changes into a single commit.

- Make prototype and function identical for gc_addspecial. Also fixes
  uncrustify misparsing the embedded function pointer decleration
- Disallow uncrustify to reformat link_socket_init_phase1, which it
  messes up
- Format the the parameters of a call of  mbedtls_ssl_tls_prf to
  be more inline with the rest of our function calls with multiple
  arguments

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/buffer.c      | 2 +-
 src/openvpn/socket.h      | 4 +++-
 src/openvpn/ssl_mbedtls.c | 8 ++++----
 3 files changed, 8 insertions(+), 6 deletions(-)

Comments

Antonio Quartulli April 17, 2020, 4:56 a.m. UTC | #1
Hi,

On 16/04/2020 13:39, Arne Schwabe wrote:
> These are small manual changes that are done to improve the code
> style and also make the result of uncrustify better without mixing
> manual changes/automatic changes into a single commit.
> 
> - Make prototype and function identical for gc_addspecial. Also fixes
>   uncrustify misparsing the embedded function pointer decleration
> - Disallow uncrustify to reformat link_socket_init_phase1, which it
>   messes up
> - Format the the parameters of a call of  mbedtls_ssl_tls_prf to
>   be more inline with the rest of our function calls with multiple
>   arguments
> 
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>

[CUT]

> -
> +/* *INDENT-OFF* uncrustify misparses this function declarion because of
> + * embedded #if/#endif tell it to skip this section */
>  void
>  link_socket_init_phase1(struct link_socket *sock,
>                          const char *local_host,
> @@ -327,6 +328,7 @@ link_socket_init_phase1(struct link_socket *sock,
>                          int mark,
>                          struct event_timeout *server_poll_timeout,
>                          unsigned int sockflags);
> +/* Reenable uncrustify *INDENT-ON* */

I hate the idea that we have to live with these markers in the code, but
it seems the most reasonable option for now.

Later on we should better refactor this function, because if it's
confusing for the parser...it means it's confusing -period-

>  
>  void link_socket_init_phase2(struct link_socket *sock,
>                               const struct frame *frame,
> diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
> index 4f194ad7..d585111b 100644
> --- a/src/openvpn/ssl_mbedtls.c
> +++ b/src/openvpn/ssl_mbedtls.c
> @@ -209,10 +209,10 @@ int mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned char *ms,
>      memcpy(client_server_random + 32, server_random, 32);
>  
>      const size_t ms_len = sizeof(ks_ssl->ctx->session->master);
> -    int ret = mbedtls_ssl_tls_prf(
> -            tls_prf_type, ms, ms_len, session->opt->ekm_label,
> -            client_server_random, sizeof(client_server_random),
> -            ks_ssl->exported_key_material, session->opt->ekm_size);
> +    int ret = mbedtls_ssl_tls_prf(tls_prf_type, ms, ms_len,
> +                                  session->opt->ekm_label, client_server_random,
> +                                  sizeof(client_server_random), ks_ssl->exported_key_material,
> +                                  session->opt->ekm_size);
>  
>      if (!mbed_ok(ret))
>      {
> 


Other than that it looks good!

Acked-by: Antonio Quartulli <antonio@openvpn.net>

(I am no sure this can be merged without 1/3 applied, but I am acking
anyway)

Regards,
Gert Doering April 19, 2020, 12:32 a.m. UTC | #2
Your patch has been applied to the master branch.

(And yes...)

commit cbde07f474ae9e92b329475767c4660dd35b4ee4
Author: Arne Schwabe
Date:   Thu Apr 16 13:39:29 2020 +0200

     Minor style change to improve code style

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Antonio Quartulli <antonio@openvpn.net>
     Message-Id: <20200416113930.15192-2-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19748.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index 8575e295..b32bc8b2 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -474,7 +474,7 @@  x_gc_freespecial(struct gc_arena *a)
 }
 
 void
-gc_addspecial(void *addr, void (free_function)(void *), struct gc_arena *a)
+gc_addspecial(void *addr, void (*free_function)(void *), struct gc_arena *a)
 {
     ASSERT(a);
     struct gc_entry_special *e;
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index e95547d1..38e5138d 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -296,7 +296,8 @@  int openvpn_connect(socket_descriptor_t sd,
 /*
  * Initialize link_socket object.
  */
-
+/* *INDENT-OFF* uncrustify misparses this function declarion because of
+ * embedded #if/#endif tell it to skip this section */
 void
 link_socket_init_phase1(struct link_socket *sock,
                         const char *local_host,
@@ -327,6 +328,7 @@  link_socket_init_phase1(struct link_socket *sock,
                         int mark,
                         struct event_timeout *server_poll_timeout,
                         unsigned int sockflags);
+/* Reenable uncrustify *INDENT-ON* */
 
 void link_socket_init_phase2(struct link_socket *sock,
                              const struct frame *frame,
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 4f194ad7..d585111b 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -209,10 +209,10 @@  int mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned char *ms,
     memcpy(client_server_random + 32, server_random, 32);
 
     const size_t ms_len = sizeof(ks_ssl->ctx->session->master);
-    int ret = mbedtls_ssl_tls_prf(
-            tls_prf_type, ms, ms_len, session->opt->ekm_label,
-            client_server_random, sizeof(client_server_random),
-            ks_ssl->exported_key_material, session->opt->ekm_size);
+    int ret = mbedtls_ssl_tls_prf(tls_prf_type, ms, ms_len,
+                                  session->opt->ekm_label, client_server_random,
+                                  sizeof(client_server_random), ks_ssl->exported_key_material,
+                                  session->opt->ekm_size);
 
     if (!mbed_ok(ret))
     {