polarssl: remove code duplication in key_state_write_plaintext{, _const}()

PATCH In review Steffan Karger 2 participants 5 messages May 3, 2015

Share & tools

Steffan Karger

author

2015-05-03 09:23 UTC #0 lore raw

PATCH 1/1 polarssl: remove code duplication in key_state_write_plaintext{, _const}()

Both functions had almost exactly the same code. Instead of the code duplication, have key_state_write_plaintext() call key_state_write_plaintext_const() to do the actual work. This is just a bit of cleanup, it should not change any behaviour.

Signed-off-by Steffan Karger <[email protected]...>

---
 src/openvpn/ssl_polarssl.c | 39 +++++----------------------------------
 1 file changed, 5 insertions(+), 34 deletions(-)

src/openvpn/ssl_polarssl.c +5 ~~-34~~

index d024e13..1a40304 100644
@@ -828,47 +828,18 @@ int
 key_state_write_plaintext (struct key_state_ssl *ks, struct buffer *buf)
 {
   int retval = 0;
-  perf_push (PERF_BIO_WRITE_PLAINTEXT);
 
-  ASSERT (NULL != ks);
   ASSERT (buf);
-  ASSERT (buf->len >= 0);
-
-  if (0 == buf->len)
-    {
-      perf_pop ();
-      return 0;
-    }
 
-  retval = ssl_write(ks->ctx, BPTR(buf), buf->len);
+  retval = key_state_write_plaintext_const(ks, BPTR(buf), BLEN(buf));
 
-  if (retval < 0)
+  if (1 == retval)
     {
-      perf_pop ();
-      if (POLARSSL_ERR_NET_WANT_WRITE == retval || POLARSSL_ERR_NET_WANT_READ == retval)
-	return 0;
-      polar_log_err (D_TLS_ERRORS, retval,
-	  "TLS ERROR: write tls_write_plaintext error");
-      return -1;
-    }
-
-  if (retval != buf->len)
-    {
-      msg (D_TLS_ERRORS,
-	  "TLS ERROR: write tls_write_plaintext incomplete %d/%d",
-	  retval, buf->len);
-      perf_pop ();
-      return -1;
+      memset (BPTR (buf), 0, BLEN (buf)); /* erase data just written */
+      buf->len = 0;
     }
 
-  /* successful write */
-  dmsg (D_HANDSHAKE_VERBOSE, "write tls_write_plaintext %d bytes", retval);
-
-  memset (BPTR (buf), 0, BLEN (buf)); /* erase data just written */
-  buf->len = 0;
-
-  perf_pop ();
-  return 1;
+  return retval;
 }
 
 int

Signature · 1 line

2.1.4

Gert Doering

2015-05-03 11:51 UTC #1 lore raw

Hi,

Quoted 4 lines · Steffan Karger

Both functions had almost exactly the same code. Instead of the code duplication, have key_state_write_plaintext() call key_state_write_plaintext_const() to do the actual work. This is just a bit of cleanup, it should not change any behaviour.

I'm somewhat lazy so I just complain instead of searching... "it does not apply for me".

Applying: polarssl: remove code duplication in key_state_write_plaintext{, _const}()
error: patch failed: src/openvpn/ssl_polarssl.c:828
error: src/openvpn/ssl_polarssl.c: patch does not apply
Patch failed at 0001 polarssl: remove code duplication in key_state_write_plaintext{, _const}()

(most likely the context differs enough in your tree - could you rebase to master and resend, please?)

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Steffan Karger

author

2015-05-03 12:06 UTC #2 lore raw

[Openvpn-devel] (no subject)

Sorry, I indeed have a few more commits in between in my local tree. In particaly, the polarssl logging improvements. Attached a rebase version of the previous patch.

(Let's see if I got the reply-to stuff in git send-email correct...)

-Steffan

Steffan Karger

author

2015-05-03 12:06 UTC #3 lore raw

PATCH 1/1 polarssl: remove code duplication in key_state_write_plaintext{, _const}()

Signed-off-by Steffan Karger <[email protected]...>

---
 src/openvpn/ssl_polarssl.c | 38 +++++---------------------------------
 1 file changed, 5 insertions(+), 33 deletions(-)

src/openvpn/ssl_polarssl.c +5 ~~-33~~

index 8cb328e..913585d 100644
@@ -838,46 +838,18 @@ int
 key_state_write_plaintext (struct key_state_ssl *ks, struct buffer *buf)
 {
   int retval = 0;
-  perf_push (PERF_BIO_WRITE_PLAINTEXT);
 
-  ASSERT (NULL != ks);
   ASSERT (buf);
-  ASSERT (buf->len >= 0);
 
-  if (0 == buf->len)
-    {
-      perf_pop ();
-      return 0;
-    }
-
-  retval = ssl_write(ks->ctx, BPTR(buf), buf->len);
-
-  if (retval < 0)
-    {
-      perf_pop ();
-      if (POLARSSL_ERR_NET_WANT_WRITE == retval || POLARSSL_ERR_NET_WANT_READ == retval)
-	return 0;
-      msg (D_TLS_ERRORS, "TLS ERROR: write tls_write_plaintext error");
-      return -1;
-    }
+  retval = key_state_write_plaintext_const(ks, BPTR(buf), BLEN(buf));
 
-  if (retval != buf->len)
+  if (1 == retval)
     {
-      msg (D_TLS_ERRORS,
-	  "TLS ERROR: write tls_write_plaintext incomplete %d/%d",
-	  retval, buf->len);
-      perf_pop ();
-      return -1;
+      memset (BPTR (buf), 0, BLEN (buf)); /* erase data just written */
+      buf->len = 0;
     }
 
-  /* successful write */
-  dmsg (D_HANDSHAKE_VERBOSE, "write tls_write_plaintext %d bytes", retval);
-
-  memset (BPTR (buf), 0, BLEN (buf)); /* erase data just written */
-  buf->len = 0;
-
-  perf_pop ();
-  return 1;
+  return retval;
 }
 
 int

Signature · 1 line

2.1.4

Gert Doering

2015-05-03 12:43 UTC #4 lore raw

ACK.

Your patch has been applied to the master branch (doesn't want to be cherry-picked into release/2.3)

commit 23b6ba6378bf3a3f5ceb828c8a4dd7cc38947d07 (master)

Author: Steffan Karger
Date:   Sun May 3 14:06:01 2015 +0200

     polarssl: remove code duplication in key_state_write_plaintext{, _const}()

     Signed-off-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/9640
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering