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

openssl 1.1.0c raises compiler warning on DEFINE_LHASH_OF(OPENSSL_CSTRING); #2214

Closed
kdekker opened this issue Jan 11, 2017 · 9 comments

Comments

Projects
None yet
5 participants
@kdekker
Copy link

commented Jan 11, 2017

OpenSSL 1.1.0c raises a compiler warning (that in our case was turned into an error) on:

openssl\lhash.h(198): error C4090: 'function': different 'const' qualifiers

This can be fixed by adding an const qualifier to OPENSSL_LH_insert() in the same header file. The diff is then:

75c75
< void *OPENSSL_LH_insert(OPENSSL_LHASH *lh, void *data);
---
> void *OPENSSL_LH_insert(OPENSSL_LHASH *lh, const void *data);

Adding this, will solve a lot of compiler warning. The cause for this warning is that OPENSSL_CSTRING is already const (char *).

Please let me know whether this information is enough to fix this issue. If needed, I can deliver a diff in patch format.

@richsalz

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

That change isn't sufficient, since the declaration and the function definition won't agree and the file will fail to compile. A more complete PR that added const more completely would be useful, but not clear if we'd do it for 1.1.1 as there isn't consistency on what causes an ABI breakage.

@kdekker

This comment has been minimized.

Copy link
Author

commented Jan 11, 2017

@richsalz

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2017

We don't do cURL. :) A PR is a pull request. See the gihub overview docs.
It might be that the most useful way to address this is to turn off C4090 as an error for files that use the openssl stuff.

@kdekker

This comment has been minimized.

Copy link
Author

commented Jan 11, 2017

@kdekker

This comment has been minimized.

Copy link
Author

commented Jan 12, 2017

@richsalz

This comment has been minimized.

Copy link
Contributor

commented Jan 12, 2017

@kdekker

This comment has been minimized.

Copy link
Author

commented Feb 8, 2017

Just curious: will anyone look for it to solve this? Or do I something else to mark this as bug?
If you like, I can make a reproduction. But the root cause (at our side) is that warning C4090 is turned into an error (with reason, for all our code). I can suppress this for OpenSSL, but still find this a bug...

@levitte levitte self-assigned this Feb 8, 2017

qloong added a commit to tianocore/edk2 that referenced this issue Mar 29, 2017

CryptoPkg: Add extra build option to disable VS build warning
openssl/include/openssl/lhash.h will bring C4090 build warning
issue, which is one known issue for OpenSSL under Visual Studio
toolchain.
Refer to openssl/openssl#2214 for more
discussions against this.
Use /wd4090 to silence this build warning until OpenSSL fix this.

Cc: Ting Ye <ting.ye@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gary Lin <glin@suse.com>
Cc: Ronald Cron <ronald.cron@arm.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <qin.long@intel.com>
Reviewed-by: Ting Ye <ting.ye@intel.com>
@FdaSilvaYY

This comment has been minimized.

Copy link
Contributor

commented May 9, 2017

See #1939 and #3420

@mattcaswell

This comment has been minimized.

Copy link
Member

commented Jan 18, 2018

Looks like this was fixed by #3420.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.