icinga2 icon indicating copy to clipboard operation
icinga2 copied to clipboard

Fix compiler warnings

Open Al2Klimov opened this issue 2 years ago • 12 comments

Al2Klimov avatar Jun 22 '22 17:06 Al2Klimov

@julianbrost Would you prefer me finish this or have this merged "ASAP" not to have it laying around for too long?

Al2Klimov avatar Jul 25 '22 15:07 Al2Klimov

Btw. any idea what's the matter with the unused OpenSSL functions?

Al2Klimov avatar Jul 25 '22 16:07 Al2Klimov

TODO

  • [x] Have a look at Windows

Al2Klimov avatar Jul 25 '22 17:07 Al2Klimov

@julianbrost Would you prefer me finish this or have this merged "ASAP" not to have it laying around for too long?

What are the thing you would or wouldn't do in each case?

But I'd rather keep this PR at a reviewable size rather than growing it to some monster nobody got time for to review.

Btw. any idea what's the matter with the unused OpenSSL functions?

Which ones do you mean?

julianbrost avatar Jul 26 '22 07:07 julianbrost

@julianbrost Would you prefer me finish this or have this merged "ASAP" not to have it laying around for too long?

What are the thing you would or wouldn't do in each case?

But I'd rather keep this PR at a reviewable size rather than growing it to some monster nobody got time for to review.

I'm done anyway with not Windows-specific stuff.

Btw. any idea what's the matter with the unused OpenSSL functions?

Which ones do you mean?

lib/base/tlsutility.cpp:33:13: warning: unused function 'OpenSSLLockingCallback' [-Wunused-function]
lib/base/tlsutility.cpp:41:22: warning: unused function 'OpenSSLIDCallback' [-Wunused-function]

Al2Klimov avatar Jul 26 '22 09:07 Al2Klimov

Btw. any idea what's the matter with the unused OpenSSL functions?

Which ones do you mean?

lib/base/tlsutility.cpp:33:13: warning: unused function 'OpenSSLLockingCallback' [-Wunused-function]
lib/base/tlsutility.cpp:41:22: warning: unused function 'OpenSSLIDCallback' [-Wunused-function]

They are used with some functions up-to-date OpenSSL versions no longer use. So figure out if it's still used in any OpenSSL version we have to support and if so, put it in #ifdef, otherwise remove it.

julianbrost avatar Jul 26 '22 09:07 julianbrost

Oh, that's the point! They are inside #ifdefs... but reported as unused?!

Al2Klimov avatar Jul 26 '22 09:07 Al2Klimov

They are inside #ifdef CRYPTO_LOCK, not some version check. So probably some "do it if that version of OpenSSL already supports it", now it's no longer used and some defines were left there so that they don't actively break code.

julianbrost avatar Jul 26 '22 09:07 julianbrost

To me they seem to be still used. (Just search for them via Ctrl+F in the same file.) And the usages are in identical ifdefs.

Al2Klimov avatar Jul 26 '22 09:07 Al2Klimov

Used with this macro that expands to nothing (i.e. no use of the function):

#  define CRYPTO_set_locking_callback(func)

Which is inside:

# if OPENSSL_API_COMPAT < 0x10100000L

So looks like that function was deprecated with 1.1.0, and I think CentOS 7 still uses an older version (:

julianbrost avatar Jul 26 '22 09:07 julianbrost

Cool. /s

Al2Klimov avatar Jul 26 '22 10:07 Al2Klimov

Actually this PR has never been about compiler warnings cleanup by itself. I as a dev just don’t want to get spammed with them as that hides everything else. I consider the current amount of warnings just fine. Apropos! Do you remember my TODO list? Its only point is checkboxed because I actually took a look at Windows (compiler warnings). Not more and not less. I decided not to care (for now), primarily to get this done. So no reason to overengineer anything.

Al2Klimov avatar Jul 28 '22 16:07 Al2Klimov

Or to say it with your wording:

But I'd rather keep this PR at a reviewable size rather than growing it to some monster nobody got time for to review.

Al2Klimov avatar Mar 23 '23 11:03 Al2Klimov

By the way. Depending on the bison/flex version one can get these warnings:

build/tools/mkclass/class_lexer.cc:2090:58: warning: unused parameter 'yyscanner' [-Wunused-parameter]
static void yy_fatal_error (yyconst char* msg , yyscan_t yyscanner)
                                                         ^
build/tools/mkclass/class_lexer.cc:2434:43: warning: unused parameter 'yyscanner' [-Wunused-parameter]
void *yyalloc (yy_size_t  size , yyscan_t yyscanner)
                                          ^
build/tools/mkclass/class_lexer.cc:2439:58: warning: unused parameter 'yyscanner' [-Wunused-parameter]
void *yyrealloc  (void * ptr, yy_size_t  size , yyscan_t yyscanner)
                                                         ^
build/tools/mkclass/class_lexer.cc:2451:36: warning: unused parameter 'yyscanner' [-Wunused-parameter]
void yyfree (void * ptr , yyscan_t yyscanner)

@yhabteab You as a Mac user can fix these easily:

  • brew install bison
  • brew install flex
  • cmake: add -DBISON_EXECUTABLE=/usr/local/Cellar/bison/3.8.2/bin/bison -DFLEX_EXECUTABLE=/usr/local/Cellar/flex/2.6.4_2/bin/flex

Al2Klimov avatar Mar 23 '23 12:03 Al2Klimov

Used with this macro that expands to nothing (i.e. no use of the function):

#  define CRYPTO_set_locking_callback(func)

Which is inside:

# if OPENSSL_API_COMPAT < 0x10100000L

So looks like that function was deprecated with 1.1.0, and I think CentOS 7 still uses an older version (:

Yes, 1.0.2k. Good to know. https://github.com/openssl/openssl/issues/1260#issuecomment-405846524

Al2Klimov avatar Mar 24 '23 12:03 Al2Klimov