Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sectransp.c compiler errors since #8180 #8197

Closed
bagder opened this issue Dec 28, 2021 · 8 comments
Closed

sectransp.c compiler errors since #8180 #8197

bagder opened this issue Dec 28, 2021 · 8 comments

Comments

@bagder
Copy link
Member

bagder commented Dec 28, 2021

In #8180 we removed != NULL from the checks and now clang on macOS instead reports the errors below. It is not clear to me why.

vtls/sectransp.c:1000:6: error: address of function 'SecCertificateCopyLongDescription' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  if(SecCertificateCopyLongDescription)
  ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vtls/sectransp.c:1000:6: note: prefix with the address-of operator to silence this warning
  if(SecCertificateCopyLongDescription)
     ^
     &
vtls/sectransp.c:1007:6: error: address of function 'SecCertificateCopySubjectSummary' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  if(SecCertificateCopySubjectSummary)
  ~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vtls/sectransp.c:1007:6: note: prefix with the address-of operator to silence this warning
  if(SecCertificateCopySubjectSummary)
     ^
     &
vtls/sectransp.c:1121:6: error: address of function 'SecItemCopyMatching' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  if(SecItemCopyMatching && kSecClassIdentity) {
     ^~~~~~~~~~~~~~~~~~~ ~~
vtls/sectransp.c:1121:6: note: prefix with the address-of operator to silence this warning
  if(SecItemCopyMatching && kSecClassIdentity) {
     ^
     &
vtls/sectransp.c:1409:6: error: address of function 'SSLSetProtocolVersionMax' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  if(SSLSetProtocolVersionMax) {
  ~~ ^~~~~~~~~~~~~~~~~~~~~~~~
vtls/sectransp.c:1409:6: note: prefix with the address-of operator to silence this warning
  if(SSLSetProtocolVersionMax) {
     ^
     &
vtls/sectransp.c:1691:6: error: address of function 'SSLCreateContext' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  if(SSLCreateContext) {  /* use the newer API if available */
  ~~ ^~~~~~~~~~~~~~~~
vtls/sectransp.c:1691:6: note: prefix with the address-of operator to silence this warning
  if(SSLCreateContext) {  /* use the newer API if available */
     ^
     &
vtls/sectransp.c:1725:6: error: address of function 'SSLSetProtocolVersionMax' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  if(SSLSetProtocolVersionMax) {
  ~~ ^~~~~~~~~~~~~~~~~~~~~~~~
vtls/sectransp.c:1725:6: note: prefix with the address-of operator to silence this warning
  if(SSLSetProtocolVersionMax) {
     ^
     &
vtls/sectransp.c:1983:6: error: address of function 'SSLSetSessionOption' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  if(SSLSetSessionOption && darwinver_maj >= 13) {
     ^~~~~~~~~~~~~~~~~~~ ~~
vtls/sectransp.c:1983:6: note: prefix with the address-of operator to silence this warning
  if(SSLSetSessionOption && darwinver_maj >= 13) {
     ^
     &
vtls/sectransp.c:2068:6: error: address of function 'SSLSetSessionOption' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  if(SSLSetSessionOption) {
  ~~ ^~~~~~~~~~~~~~~~~~~
vtls/sectransp.c:2068:6: note: prefix with the address-of operator to silence this warning
  if(SSLSetSessionOption) {
     ^
     &
vtls/sectransp.c:2950:6: error: address of function 'SecTrustEvaluateAsync' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  if(SecTrustEvaluateAsync) {
  ~~ ^~~~~~~~~~~~~~~~~~~~~
vtls/sectransp.c:2950:6: note: prefix with the address-of operator to silence this warning
  if(SecTrustEvaluateAsync) {
     ^
     &
vtls/sectransp.c:3168:8: error: address of function 'SSLCreateContext' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
    if(SSLCreateContext)
    ~~ ^~~~~~~~~~~~~~~~
vtls/sectransp.c:3168:8: note: prefix with the address-of operator to silence this warning
    if(SSLCreateContext)
       ^
       &
vtls/sectransp.c:3332:6: error: address of function 'SSLSetSessionOption' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion]
  if(SSLSetSessionOption)
  ~~ ^~~~~~~~~~~~~~~~~~~
vtls/sectransp.c:3332:6: note: prefix with the address-of operator to silence this warning
  if(SSLSetSessionOption)
     ^
     &
11 errors generated.
@jay
Copy link
Member

jay commented Dec 28, 2021

however it's being compiled I guess those function addresses are always going to evaluate as true. why it warns on that particular style but not if(func != NULL) or if(&func) I don't know. we had a similar constant expression warning with visual studio a while back and i disabled it entirely 9c1806a

@bagder
Copy link
Member Author

bagder commented Dec 28, 2021

I believe adding -Wno-pointer-bool-conversion will do the job.

bagder added a commit that referenced this issue Dec 28, 2021
To hush compiler warnings we don't care for: error: address of function
'X' will always evaluate to 'true'

Fixes #8197
Closes #....
@gvanem
Copy link
Contributor

gvanem commented Dec 29, 2021

why it warns on that particular style but not if(func != NULL) or if(&func) I don't know.

Probably since those functions always exist and returns CFString values. Which could include a CFNull.

@bagder
Copy link
Member Author

bagder commented Dec 29, 2021

My assumption has been that they aren't always present and that's why the check is there. Because why else check for it?

@bagder bagder closed this as completed in e902183 Dec 29, 2021
@nickzman
Copy link
Member

The checks for null in sectransp.c are there to check for a function's presence for weak-linking on old iOS and macOS versions. They have to be there; weak-linking doesn't work without the explicit check for null.

A lot of those checks are quite old, though, and I wonder if many people care if we still support macOS 10.6. 10.6 was released over twelve years ago.

@bagder
Copy link
Member Author

bagder commented Dec 30, 2021

I wonder if many people care if we still support macOS 10.6. 10.6 was released over twelve years ago.

Everything older than 10 years are usually safe to drop, I think. Often we can draw the line earlier as well.

@mback2k
Copy link
Member

mback2k commented Jan 2, 2022

As mentioned in 21248e0#r62803533 this is also now triggering warning C4706: assignment within conditional expression.

@bagder
Copy link
Member Author

bagder commented Jan 2, 2022

That is not the only assignment we do within a conditional expression in the source code, and the assignment was already there before we removed != NULL !!

Can we perhaps just silence warning C4706 with MSVC?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

5 participants