Hi, On 16/08/18 19:08, Matthias Andree wrote: > Am 15.08.2018 um 08:12 schrieb Antonio Quartulli: >> The "cleanup" label in ssl_verify.c:verify_user_pass_plugin() is used >> only when PLUGIN_DEF_AUTH is defined, therefore make the label >> definition dependent on the same define. >> >> Fixes the following warning when PLUGIN_DEF_AUTH is not defined: >> >> ssl_verify.c: In function 'verify_user_pass_plugin': >> ssl_verify.c:1223:1: warning: label 'cleanup' defined but not used >> [-Wunused-label] >> cleanup: >> ^~~~~~~ >> >> Signed-off-by: Antonio Quartulli <a...@unstable.cc> >> --- >> src/openvpn/ssl_verify.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c >> index 61872251..5b46ea72 100644 >> --- a/src/openvpn/ssl_verify.c >> +++ b/src/openvpn/ssl_verify.c >> @@ -1220,7 +1220,9 @@ verify_user_pass_plugin(struct tls_session *session, >> const struct user_pass *up, >> msg(D_TLS_ERRORS, "TLS Auth Error (verify_user_pass_plugin): peer >> provided a blank username"); >> } >> >> +#ifdef PLUGIN_DEF_AUTH >> cleanup: >> +#endif >> return retval; >> } >> > > NAK. > > This is useless #ifdef noise in the code and makes it harder to read, > for compile-time cosmetics. > > If you don't want the warnings, feed a proper -Wno-unused-label to your > compiler and move on.
Thanks for taking the time to look at this! Actually we are in the process of getting rid of the various ifdef around the code or at least we are trying to bring that to a more reasonable state (it takes time....and right now we are more focussed on the networking and platform side of things). I totally agree with you that more ifdefs are just ugly and make the code worse. However this "new" ifdef is just part of some logic that is already there (a few lines above). This said, in the past months we have tried moving towards a 0-warnings-tolerance, so that we could catch as many bugs as possible hidden by "just annoying warnings". For this reason using "-Wno-unused-label" is not an option (especially in CI/automated build tests where this was spotted). Imho we should just get this patch through and then try to get rid of the whole #ifdef PLUGIN_DEF_AUTH logic at once. Cheers, -- Antonio Quartulli
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
