Skip to contentSkip to conversation

Fix compilation in pedantic mode

PATCH In review Lev Stipakov 6 participants 11 messages Oct 4, 2016 – Oct 10, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix compilation in pedantic mode

Replace C++ style comments, which are not allowed in ISO C90 standard, with C style comments

---
 src/openvpn/crypto_openssl.c |  4 ++--
 src/openvpn/ssl.c            |  2 +-
 src/openvpn/ssl_openssl.c    | 36 ++++++++++++++++++------------------
 src/plugins/auth-pam/utils.c |  2 +-
 4 files changed, 22 insertions(+), 22 deletions(-)
src/openvpn/crypto_openssl.c +2 -2
index 2180b1a..7b1ac6c 100644
@@ -278,13 +278,13 @@ crypto_init_dmalloc (void)
 
 const char *
 translate_cipher_name_from_openvpn (const char *cipher_name) {
-  // OpenSSL doesn't require any translation
+  /* OpenSSL doesn't require any translation */
   return cipher_name;
 }
 
 const char *
 translate_cipher_name_to_openvpn (const char *cipher_name) {
-  // OpenSSL doesn't require any translation
+  /* OpenSSL doesn't require any translation */
   return cipher_name;
 }
 
src/openvpn/ssl.c +1 -1
index 4de3354..33fd9dd 100644
@@ -267,7 +267,7 @@ tls_get_cipher_name_pair (const char * cipher_name, size_t len) {
       pair++;
   }
 
-  // No entry found, return NULL
+  /* No entry found, return NULL */
   return NULL;
 }
 
src/openvpn/ssl_openssl.c +18 -18
index a226aac..2d06da9 100644
@@ -294,7 +294,7 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 
   ASSERT(NULL != ctx);
 
-  // Translate IANA cipher suite names to OpenSSL names
+  /* Translate IANA cipher suite names to OpenSSL names */
   begin_of_cipher = end_of_cipher = 0;
   for (; begin_of_cipher < strlen(ciphers); begin_of_cipher = end_of_cipher) {
       end_of_cipher += strcspn(&ciphers[begin_of_cipher], ":");
@@ -302,31 +302,31 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 
       if (NULL == cipher_pair)
         {
-          // No translation found, use original
-          current_cipher = &ciphers[begin_of_cipher];
-          current_cipher_len = end_of_cipher - begin_of_cipher;
-
-          // Issue warning on missing translation
-          // %.*s format specifier expects length of type int, so guarantee
-          // that length is small enough and cast to int.
-          msg (M_WARN, "No valid translation found for TLS cipher '%.*s'",
-              (int) MIN(current_cipher_len, 256), current_cipher);
+	  /* No translation found, use original */
+	  current_cipher = &ciphers[begin_of_cipher];
+	  current_cipher_len = end_of_cipher - begin_of_cipher;
+
+	  /* Issue warning on missing translation
+	     %.*s format specifier expects length of type int, so guarantee
+	     that length is small enough and cast to int. */
+	  msg (M_WARN, "No valid translation found for TLS cipher '%.*s'",
+	      (int) MIN(current_cipher_len, 256), current_cipher);
         }
       else
 	{
-	  // Use OpenSSL name
-          current_cipher = cipher_pair->openssl_name;
-          current_cipher_len = strlen(current_cipher);
+	  /* Use OpenSSL name */
+	  current_cipher = cipher_pair->openssl_name;
+	  current_cipher_len = strlen(current_cipher);
 
 	  if (end_of_cipher - begin_of_cipher == current_cipher_len &&
 	      0 == memcmp (&ciphers[begin_of_cipher], cipher_pair->openssl_name, end_of_cipher - begin_of_cipher))
 	    {
-	      // Non-IANA name used, show warning
+	      /* Non-IANA name used, show warning */
 	      msg (M_WARN, "Deprecated TLS cipher name '%s', please use IANA name '%s'", cipher_pair->openssl_name, cipher_pair->iana_name);
 	    }
 	}
 
-      // Make sure new cipher name fits in cipher string
+      /* Make sure new cipher name fits in cipher string */
       if (((sizeof(openssl_ciphers)-1) - openssl_ciphers_len) < current_cipher_len)
 	{
 	  msg (M_FATAL,
@@ -334,7 +334,7 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 	      (int)sizeof(openssl_ciphers)-1);
 	}
 
-      // Concatenate cipher name to OpenSSL cipher string
+      /* Concatenate cipher name to OpenSSL cipher string */
       memcpy(&openssl_ciphers[openssl_ciphers_len], current_cipher, current_cipher_len);
       openssl_ciphers_len += current_cipher_len;
       openssl_ciphers[openssl_ciphers_len] = ':';
@@ -346,7 +346,7 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
   if (openssl_ciphers_len > 0)
     openssl_ciphers[openssl_ciphers_len-1] = '\0';
 
-  // Set OpenSSL cipher list
+  /* Set OpenSSL cipher list */
   if(!SSL_CTX_set_cipher_list(ctx->ctx, openssl_ciphers))
     crypto_msg (M_FATAL, "Failed to set restricted TLS cipher list: %s", openssl_ciphers);
 }
@@ -1407,7 +1407,7 @@ show_available_tls_ciphers (const char *cipher_list)
       pair = tls_get_cipher_name_pair(cipher_name, strlen(cipher_name));
 
       if (NULL == pair) {
-          // No translation found, print warning
+          /* No translation found, print warning */
 	  printf ("%s (No IANA name known to OpenVPN, use OpenSSL name.)\n", cipher_name);
       } else {
 	  printf ("%s\n", pair->iana_name);
src/plugins/auth-pam/utils.c +1 -1
index 4f2bec1..e4643cd 100644
@@ -59,7 +59,7 @@ searchandreplace(const char *tosearch, const char *searchfor, const char *replac
        return NULL;
   }
 
-  // state: all parameters are valid
+  /* state: all parameters are valid */
 
   const char *searching=tosearch;
   char *scratch;
Signature · 1 line
1.9.1
#1 lore raw

Hi,

Quoted 135 lines · Lev Stipakov

Replace C++ style comments, which are not allowed in ISO C90 standard, with C style comments

---
 src/openvpn/crypto_openssl.c |  4 ++--
 src/openvpn/ssl.c            |  2 +-
 src/openvpn/ssl_openssl.c    | 36 ++++++++++++++++++------------------
 src/plugins/auth-pam/utils.c |  2 +-
 4 files changed, 22 insertions(+), 22 deletions(-)
src/openvpn/crypto_openssl.c +2 -2
index 2180b1a..7b1ac6c 100644
@@ -278,13 +278,13 @@ crypto_init_dmalloc (void)
 
 const char *
 translate_cipher_name_from_openvpn (const char *cipher_name) {
-  // OpenSSL doesn't require any translation
+  /* OpenSSL doesn't require any translation */
   return cipher_name;
 }
 
 const char *
 translate_cipher_name_to_openvpn (const char *cipher_name) {
-  // OpenSSL doesn't require any translation
+  /* OpenSSL doesn't require any translation */
   return cipher_name;
 }
 
src/openvpn/ssl.c +1 -1
index 4de3354..33fd9dd 100644
@@ -267,7 +267,7 @@ tls_get_cipher_name_pair (const char * cipher_name, size_t len) {
       pair++;
   }
 
-  // No entry found, return NULL
+  /* No entry found, return NULL */
   return NULL;
 }
 
src/openvpn/ssl_openssl.c +18 -18
index a226aac..2d06da9 100644
@@ -294,7 +294,7 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 
   ASSERT(NULL != ctx);
 
-  // Translate IANA cipher suite names to OpenSSL names
+  /* Translate IANA cipher suite names to OpenSSL names */
   begin_of_cipher = end_of_cipher = 0;
   for (; begin_of_cipher < strlen(ciphers); begin_of_cipher = end_of_cipher) {
       end_of_cipher += strcspn(&ciphers[begin_of_cipher], ":");
@@ -302,31 +302,31 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 
       if (NULL == cipher_pair)
         {
-          // No translation found, use original
-          current_cipher = &ciphers[begin_of_cipher];
-          current_cipher_len = end_of_cipher - begin_of_cipher;
-
-          // Issue warning on missing translation
-          // %.*s format specifier expects length of type int, so guarantee
-          // that length is small enough and cast to int.
-          msg (M_WARN, "No valid translation found for TLS cipher '%.*s'",
-              (int) MIN(current_cipher_len, 256), current_cipher);
+	  /* No translation found, use original */
+	  current_cipher = &ciphers[begin_of_cipher];
+	  current_cipher_len = end_of_cipher - begin_of_cipher;
+
+	  /* Issue warning on missing translation
+	     %.*s format specifier expects length of type int, so guarantee
+	     that length is small enough and cast to int. */
+	  msg (M_WARN, "No valid translation found for TLS cipher '%.*s'",
+	      (int) MIN(current_cipher_len, 256), current_cipher);
         }
       else
 	{
-	  // Use OpenSSL name
-          current_cipher = cipher_pair->openssl_name;
-          current_cipher_len = strlen(current_cipher);
+	  /* Use OpenSSL name */
+	  current_cipher = cipher_pair->openssl_name;
+	  current_cipher_len = strlen(current_cipher);
 
 	  if (end_of_cipher - begin_of_cipher == current_cipher_len &&
 	      0 == memcmp (&ciphers[begin_of_cipher], cipher_pair->openssl_name, end_of_cipher - begin_of_cipher))
 	    {
-	      // Non-IANA name used, show warning
+	      /* Non-IANA name used, show warning */
 	      msg (M_WARN, "Deprecated TLS cipher name '%s', please use IANA name '%s'", cipher_pair->openssl_name, cipher_pair->iana_name);
 	    }
 	}
 
-      // Make sure new cipher name fits in cipher string
+      /* Make sure new cipher name fits in cipher string */
       if (((sizeof(openssl_ciphers)-1) - openssl_ciphers_len) < current_cipher_len)
 	{
 	  msg (M_FATAL,
@@ -334,7 +334,7 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 	      (int)sizeof(openssl_ciphers)-1);
 	}
 
-      // Concatenate cipher name to OpenSSL cipher string
+      /* Concatenate cipher name to OpenSSL cipher string */
       memcpy(&openssl_ciphers[openssl_ciphers_len], current_cipher, current_cipher_len);
       openssl_ciphers_len += current_cipher_len;
       openssl_ciphers[openssl_ciphers_len] = ':';
@@ -346,7 +346,7 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
   if (openssl_ciphers_len > 0)
     openssl_ciphers[openssl_ciphers_len-1] = '\0';
 
-  // Set OpenSSL cipher list
+  /* Set OpenSSL cipher list */
   if(!SSL_CTX_set_cipher_list(ctx->ctx, openssl_ciphers))
     crypto_msg (M_FATAL, "Failed to set restricted TLS cipher list: %s", openssl_ciphers);
 }
@@ -1407,7 +1407,7 @@ show_available_tls_ciphers (const char *cipher_list)
       pair = tls_get_cipher_name_pair(cipher_name, strlen(cipher_name));
 
       if (NULL == pair) {
-          // No translation found, print warning
+          /* No translation found, print warning */
 	  printf ("%s (No IANA name known to OpenVPN, use OpenSSL name.)\n", cipher_name);
       } else {
 	  printf ("%s\n", pair->iana_name);
src/plugins/auth-pam/utils.c +0 -0
index 4f2bec1..e4643cd 100644

src/plugins/auth-pam/utils.c searchandreplace(const char *tosearch, const char *searchfor, const char *replac · line 65

Quoted 8 lines
src/plugins/auth-pam/utils.c +1 -1
@@ -59,7 +59,7 @@ searchandreplace(const char *tosearch, const char *searchfor, const char *replac
        return NULL;
   }
 
-  // state: all parameters are valid
+  /* state: all parameters are valid */
 
   const char *searching=tosearch;
   char *scratch;

I do prefer

/* Some text
* more text
*/

over

/* Some text
more text */

but am not too religious about that.

So, ACK.

-Steffan

#2 lore raw

Hi,

Quoted 6 lines · Steffan Karger

I do prefer

/* Some text
* more text
*/

Quoted 6 lines

over

/* Some text
more text */

but am not too religious about that.

I am, and the first version is what we do in (most?) other places, so I'll change that on the fly.

Also, the patch introduces spurious tab<->whitespace changes "close to comments" in a few places - will undo those, too.

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

Hi,

I am, and the first version is what we do in (most?) other places, so I'll

Quoted 2 lines

change that on the fly.

Got it.

Quoted 2 lines

Also, the patch introduces spurious tab<->whitespace changes "close to comments" in a few places - will undo those, too.

I noticed that in few places indentation is a bit off - like 8 whitespaces instead of tab so I decided to "fix" it.

Should I just ignore those things or do it in separate PR?

Signature · 1 line
-Lev
#4 lore raw

Hi,

Quoted 4 lines · Lev Stipakov

I noticed that in few places indentation is a bit off - like 8 whitespaces instead of tab so I decided to "fix" it.

Should I just ignore those things or do it in separate PR?

Please do not intermix whitespace changes with "other changes" - this makes it harder to track what really changed in a patch.

We are planning to do a massive re-indentation before 2.4 release (right when we branch off the release/2.4 branch, or so) - so I'd not bother right now.

If you do actual code changes to a line that has funny whitespace, of course feel free to fix that as well ;-)

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]...
#5 lore raw
In reply to Lev Stipakov

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 10 lines · Lev Stipakov

Hi,

I am, and the first version is what we do in (most?) other places, so I'll change that on the fly.

Got it.

Quoted 8 lines

Also, the patch introduces spurious tab<->whitespace changes "close to comments" in a few places - will undo those, too.

I noticed that in few places indentation is a bit off - like 8 whitespaces instead of tab so I decided to "fix" it.

Should I just ignore those things or do it in separate PR?

Currently I think we can leave them. Before the release of 2.4-RC we will do a lot of coding style fixes to the whole source tree, so that we have one unified coding style over the whole code base in both the new releases/2.4 and master branch.

The Hackathon in Munich 2014 [1] decided on the the Allmann style [2].
The only thing I'm not sure we agreed upon is the indent size (4 or 8
spaces), in addition to the line length which is not that explicitly agreed upon.

[1] <https://community.openvpn.net/openvpn/wiki/MunichHackathon2014>
[2] <https://en.wikipedia.org/wiki/Indent_style#Allman_style>

- --
kind regards,

David Sommerseth
OpenVPN Technologies, Inc

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJX9UWvAAoJEIbPlEyWcf3ytj4P/iuXf+htHtFjQfE9/QSWY/Ui +jpF1kqlAKac6eS1t0cN1OYVUmXZwNkZjGNKYl8vi9zVSciZf2rFslTAhaaRLuWW LMtL1MxGJbADvLpKkHPfvSNk3ITXxhnMKjgXqzxnPY2WeyV7f48jKOUeBma3RY1R m4hT8Hg0Im2cqVmU9XRDvEc3u0EqTKzA/NP6sKO1dWut05ZwKlIyUknsJfLoZyEx OPghZaTgFmz0YZKQqam0cenkL2g3cYuz60GQw8Lpio6GFLQMMbjV9fRpdIJ4d36W qIdulNh+idXDvuSe3bFFQqYItdUIIgaBvz8dUDCubz47OR6vkTD+/+KR0f4F5C/9 HHt1CW4GAs5AEvFpF9BhPoxajQHNjoZsrWOL+YgumVKcTUclgd/iUoRTBusSF87H fYp/b7XjLO1cr1QPprK0szzNQB4HMFx3aVxZlvhC8WePN+uO8a6X55g08pnuy7kS PQ5nrfOywwQsqhl9p9E5lFV2HoeIfxGkwjoq4xztGI9xyV3jFroaqn4cN9314+aF RREgmCYInDEZzVXjqFCC/0isgEXOYdC/p6Hpy6EWzCecFf8fm1Gs4CQnhodR7Min QyKZhhC/yAXgOKuEKCPw+NY+eulE1VwRmIBEJujKgeVBNVIoaQc43wEARIYhTdcO SrffianKpvhWJiPjLRUd
=TF/Z
-----END PGP SIGNATURE-----

#6 lore raw
Quoted 4 lines · David Sommerseth

The Hackathon in Munich 2014 [1] decided on the the Allmann style [2].
The only thing I'm not sure we agreed upon is the indent size (4 or 8
spaces), in addition to the line length which is not that explicitly agreed upon.

I seem to remember we agreed on 4 spaces. (8 would really not work with the current code and a 80-chars-per-line limit.)

-Steffan

#7 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 9 lines · Steffan Karger
Quoted 4 lines · David Sommerseth

The Hackathon in Munich 2014 [1] decided on the the Allmann style [2]. The only thing I'm not sure we agreed upon is the indent size (4 or 8 spaces), in addition to the line length which is not that explicitly agreed upon.

I seem to remember we agreed on 4 spaces. (8 would really not work with the current code and a 80-chars-per-line limit.)

Fair point! I remember we discussed both, but as we never truly documented any result here I have forgotten it. And it is aligned what Gert claims too.

- --
kind regards,

David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJX9UnOAAoJEIbPlEyWcf3yXA0P/AlQ/JE8G04qsS0oK3fFGRUX HfUOgza8EdCKa5VlRoS1WlNieqGzItoxhHjeX1tl2S6RSgG6JPS6sRIHm6zIjWlB S+cuVVAhRDE2kJGEYSJAUUbgBT2eQKRIYUyKjEfiUmPpOfDn5/7Bi5eWrsb+H6MD IsUw3NsE5f7SXsWrxNEUoQ/wni85EaOZsv4xuD2eElE/VLApEoYI5PheSkiJ2JIv 96Zc4t10kve0bXcIU4/Kuns+ZWp2b/znaKjjw5wAFNRm0sKIl5z3YMChUaDBrWaD JPLDLTxI6T+TmurMh2xGeuy3NCXkXq6b5p8c2jYIP8k+21mMBOzGEBUgNs+Ft/uO 0sfvT1k12hKPFujyLR0LTyOJ8nxuu6UoeKdznAqRkJb3HCgBVB0mb9OjVFK/5pQV iFwqhi1/igupy8YfxHIjb6HwgaB70hzMdzTq1dHEBpHGTRVaH91M5b8zXJHyyqNG u48SQWdv7sct+/XOTuQvceJVNqWXgGy29fR+4rNB6eS4kHzEtvacxsDfjJrvBHQd IJZOPF9bFn6p97YAMOdVzHl1iT4I/OCvLqs0GblZCs+OeT4QvBdlZkQmFjScg/+S y31R/nNWlnos7dOBcAIfMX4jjjOKincud2sfJsTx9nXdUFqZrdm7zAba+vPXJ/9E mLz4FMAiyeV6Fvn5A0vY
=QaqU
-----END PGP SIGNATURE-----

#8 lore raw

On Wednesday, October 5, 2016 12:43, David Sommerseth wrote:
On 05/10/16 20:34, Steffan Karger wrote:
> On 5 October 2016 at 20:25, David Sommerseth
> <[email protected]...> wrote:
>> The Hackathon in Munich 2014 [1] decided on the the Allmann style >> [2]. The only thing I'm not sure we agreed upon is the indent >> size (4 or 8 spaces), in addition to the line length which is not >> that explicitly agreed upon.
>
> I seem to remember we agreed on 4 spaces. (8 would really not > work with the current code and a 80-chars-per-line limit.)

Fair point! I remember we discussed both, but as we never truly documented any result here I have forgotten it. And it is aligned what Gert claims too.

Also, 8 spaces is a terrible idea anyway. Even the most ancient printing presses (after indentation was initially used) only went as far as (the equivalent of) 5 - and their fonts were proportional.

That said, I will always prefer \t over \x20*.

Daniel Hunsaker
QA / Automation
OpenVPN Technologies, Inc

irc://irc.freenode.net: danhunsaker

#9 lore raw
In reply to David Sommerseth

Hi,

Quoted 4 lines · David Sommerseth

The Hackathon in Munich 2014 [1] decided on the the Allmann style [2].
The only thing I'm not sure we agreed upon is the indent size (4 or 8
spaces), in addition to the line length which is not that explicitly agreed upon.

/me claims "it was 4" :)

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]...
#10 lore raw
In reply to Lev Stipakov

ACK. I've decided to leave the whitespace changes as they were in your patch ("git show -w" will confirm only whitespace and comments have been changed), and changed the multiline comment to "leading *" style, as discussed previously.

Your patch has been applied to the release/2.3 branch.

commit d72c3835e20593091d4d2c69466329f994b69ae6

Author: Lev Stipakov
Date:   Tue Oct 4 23:42:16 2016 +0300
     Fix compilation in pedantic mode

     Signed-off-by: Lev Stipakov <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg12600.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering