diff mbox series

[Openvpn-devel] Remove extra token after #endif

Message ID 1541757573-30178-1-git-send-email-lstipakov@gmail.com
State Accepted
Headers
Series [Openvpn-devel] Remove extra token after #endif |

Commit Message

Lev Stipakov Nov. 8, 2018, 10:59 p.m. UTC 
  Commit ee80ce3d6f2ebc59068338757311e0488ae620fc wrapped
code in #ifdef/#endif and added extra token after #endif,
which produces compiler warning.

This removes unneeded extra token.

Signed-off-by: Lev Stipakov <lstipakov@gmail.com>
---
 src/openvpn/init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Christian Hesse Nov. 8, 2018, 11:24 p.m. UTC | #1 
Lev Stipakov <lstipakov@gmail.com> on Fri, 2018/11/09 11:59:
> Commit ee80ce3d6f2ebc59068338757311e0488ae620fc wrapped
> code in #ifdef/#endif and added extra token after #endif,
> which produces compiler warning.
> 
> This removes unneeded extra token.
> 
> Signed-off-by: Lev Stipakov <lstipakov@gmail.com>
> ---
>  src/openvpn/init.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 586e4ca..1cdef31 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -624,7 +624,7 @@ save_ncp_options(struct context *c)
>      c->c1.ciphername = c->options.ciphername;
>      c->c1.authname = c->options.authname;
>      c->c1.keysize = c->options.keysize;
> -#endif ENABLE_CRYPTO
> +#endif

Perhaps this should go into comment:

#endif /* ENABLE_CRYPTO */
Gert Doering Nov. 20, 2018, 10:34 p.m. UTC | #2 
Acked-by: Gert Doering <gert@greenie.muc.de>

Thanks.

Regarding Christian's comment about "#endif" vs. "#endif /* ENABLE_CRYPTO /*"
- well, we have both in our tree, so neither is "correct" or "wrong".

For such a short #ifdef/#endif span, I'd see it as optional because the
corresponding #ifdef is easily spotted.  For something spanning more
code, possibly nested #ifdef, a comment after the #endif would definitely
be better, though.

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

commit ca962f3837ab05b0e8a4382960391f289497eaa2
Author: Lev Stipakov
Date:   Fri Nov 9 11:59:33 2018 +0200

     Remove extra token after #endif

     Signed-off-by: Lev Stipakov <lstipakov@gmail.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <1541757573-30178-1-git-send-email-lstipakov@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17883.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
David Sommerseth Nov. 22, 2018, 1:46 p.m. UTC | #3 
On 21/11/2018 10:34, Gert Doering wrote:
> 
> Regarding Christian's comment about "#endif" vs. "#endif /* ENABLE_CRYPTO /*"
> - well, we have both in our tree, so neither is "correct" or "wrong".
> 
> For such a short #ifdef/#endif span, I'd see it as optional because the
> corresponding #ifdef is easily spotted.  For something spanning more
> code, possibly nested #ifdef, a comment after the #endif would definitely
> be better, though.

Just FTR; I share this point of view.  We've not been too picky about these
#endif remarks in the past but something we saw more of after the big
reformatting commit right before the v2.4.0 release.

We used these uncrustify settings for the great reformatting patch:

-------------------
# Annotate #else and #endif statements
mod_add_long_ifdef_endif_comment=20
mod_add_long_ifdef_else_comment=5
-------------------
diff mbox series

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 586e4ca..1cdef31 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -624,7 +624,7 @@  save_ncp_options(struct context *c)
     c->c1.ciphername = c->options.ciphername;
     c->c1.authname = c->options.authname;
     c->c1.keysize = c->options.keysize;
-#endif ENABLE_CRYPTO
+#endif
 }
 
 /* Restores NCP-negotiable options to original values */
@@ -635,7 +635,7 @@  restore_ncp_options(struct context *c)
     c->options.ciphername = c->c1.ciphername;
     c->options.authname = c->c1.authname;
     c->options.keysize = c->c1.keysize;
-#endif ENABLE_CRYPTO
+#endif
 }
 
 void