Skip to contentSkip to conversation

polarssl: optimize polar_ok() for non-errors

PATCH In review Steffan Karger 3 participants 5 messages Jan 6, 2016 – Jan 7, 2016

lore
Share & tools
lore

Steffan Karger

author
#0 lore raw

PATCH 1/1 polarssl: optimize polar_ok() for non-errors

Adding polar_ok() was a good plan for improving error reporting, but also added two function calls (one to polar_log_func_line() and one to polar_log_err()) for each function call wrapped with polar_ok(). Especially in the critical path, this is a waste of time.

To avoid this overhead, add a simple check to the polar_ok() macro that reduces it to a single branch.

Signed-off-by Steffan Karger <[email protected]...>
---
 src/openvpn/crypto_polarssl.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
src/openvpn/crypto_polarssl.h +2 -2
index 92c2c30..e6db445 100644
@@ -130,8 +130,8 @@ bool polar_log_func_line(unsigned int flags, int errval, const char *func,
  *
  * @returns true if no errors are detected, false otherwise.
  */
-#define polar_ok(errval) \
-  polar_log_func_line(D_CRYPT_ERRORS, errval, __func__, __LINE__)
+#define polar_ok(errval) ((0 == errval) || \
+  polar_log_func_line(D_CRYPT_ERRORS, errval, __func__, __LINE__))
 
 
 #endif /* CRYPTO_POLARSSL_H_ */
Signature · 1 line
2.5.0
#1 lore raw

Your patch has been applied to the master branch.

commit dd8d32e1e1da90b441fb25e62a8aa0e68428f601

Author: Steffan Karger
Date:   Wed Jan 6 21:03:59 2016 +0100
     polarssl: optimize polar_ok() for non-errors

     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/10942
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

#2 lore raw

Hi,

Quoted 7 lines · Gert Doering

Your patch has been applied to the master branch.

commit dd8d32e1e1da90b441fb25e62a8aa0e68428f601

Author: Steffan Karger
Date:   Wed Jan 6 21:03:59 2016 +0100

polarssl: optimize polar_ok() for non-errors

Actually, after applying to my tree and running my test suite, preparing this mail and all, I came to the conclusion that this is not safe, as it will evaluate "errval" - which usually is a function call that might have side effects, like "destroying the original polarssl error code" - twice in case of errors.

So, *not* pusing it, resetting my local tree, and Steffan will send a new version.

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]...
#3 lore raw

PATCH v2 1/1 polarssl: optimize polar_ok() for non-errors

From: Steffan Karger <[email protected]...>

Adding polar_ok() was a good plan for improving error reporting, but also added two function calls (one to polar_log_func_line() and one to polar_log_err()) for each function call wrapped with polar_ok(). Especially in the critical path, this is a waste of time.

To avoid this overhead, add a simple static inline wrapper to reduce it to a single branch.

v2 - use a static inline wrapper to prevent evaluating 'errval' twice in
the macro.

Signed-off-by Steffan Karger <[email protected]...>
---
 src/openvpn/crypto_polarssl.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
src/openvpn/crypto_polarssl.h +10 -1
index bd0f8b8..94306ed 100644
@@ -115,6 +115,15 @@ bool polar_log_err(unsigned int flags, int errval, const char *prefix);
 bool polar_log_func_line(unsigned int flags, int errval, const char *func,
     int line);
 
+/** Wraps polar_log_func_line() to prevent function calls for non-errors */
+static inline bool polar_log_func_line_lite(unsigned int flags, int errval,
+    const char *func, int line) {
+  if (errval) {
+    return polar_log_func_line (flags, errval, func, line);
+  }
+  return true;
+}
+
 /**
  * Check errval and log on error.
  *

@@ -128,7 +137,7 @@ bool polar_log_func_line(unsigned int flags, int errval, const char *func,
  * @returns true if no errors are detected, false otherwise.
  */
 #define polar_ok(errval) \
-  polar_log_func_line(D_CRYPT_ERRORS, errval, __func__, __LINE__)
+  polar_log_func_line_lite(D_CRYPT_ERRORS, errval, __func__, __LINE__)
 
 
 #endif /* CRYPTO_POLARSSL_H_ */
Signature · 1 line
2.5.0
#4 lore raw

ACK. Slightly more work for the compiler, but supposedly they are smarter these days :-) - and actually slightly easier to read than v1.

Your patch has been applied to the master branch.

commit 3a39bf7dfe5a57fe8bc43c073b2a009bb6994e78

Author: Steffan Karger
Date:   Thu Jan 7 10:15:16 2016 +0100
     polarssl: optimize polar_ok() for non-errors

     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/10949
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering