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

Configure option 'no-deprecated' means '-DOPENSSL_API_COMPAT=0x10100000L' #6470

Closed
wants to merge 2 commits into from

Conversation

levitte
Copy link
Member

@levitte levitte commented Jun 12, 2018

(that is, until 1.2.0 comes along)

Since we allow future deprecation (and that shouldn't be affected
by 'no-deprecated'), we need to distinguish what to have deprecated
on the value of OPENSSL_API_COMPAT, not the existence of
OPENSSL_NO_DEPRECATED.

Note that the macro OPENSSL_NO_DEPRECATED still exists, in case
someone still uses it.

…00L'

(that is, until 1.2.0 comes along)

Since we allow future deprecation (and that shouldn't be affected
by 'no-deprecated'), we need to distinguish what to have deprecated
on the value of OPENSSL_API_COMPAT, not the existence of
OPENSSL_NO_DEPRECATED.

Note that the macro OPENSSL_NO_DEPRECATED still exists, in case
someone still uses it.
@levitte levitte added branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch labels Jun 12, 2018
@levitte
Copy link
Member Author

levitte commented Jun 12, 2018

There's no need to backport this to 1.1.0, since future deprecation didn't exist there.

... to the check OPENSSL_API_COMPAT < 0x10100000L, to correspond with
how it's declared.
@levitte levitte force-pushed the fix-no-deprecated-20180612 branch from 957e42b to daab428 Compare June 12, 2018 16:34
#if defined(OPENSSL_NO_DEPRECATED)
# define DECLARE_DEPRECATED(f)
#elif __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ > 0)
#if __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ > 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was report that this triggers -Wundef warning/error with clang-cl, i.e. clang which pretends to be an msc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm actually surprised that MSVC and DEC C never complained about this...
Anyway, I think that's an issue for another PR...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? Referring undefined macro is specified, so it's not like it's an error. Other compilers simply concentrate on code warnings, not pre-propcessor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just in case, it wasn't my intention to examine it thoroughly. I mean I'm not actually looking into this matter, the #if thing simply caught my eye...

@mattcaswell
Copy link
Member

#6467 is dependant on this PR - so please ping that one when you merge this.

@levitte
Copy link
Member Author

levitte commented Jun 19, 2018

Merged.

a9091c1 Convert our own check of OPENSSL_NO_DEPRECATED
973abf5 Configure option 'no-deprecated' means '-DOPENSSL_API_COMPAT=0x10100000L'

@levitte levitte closed this Jun 19, 2018
levitte added a commit that referenced this pull request Jun 19, 2018
…00L'

(that is, until 1.2.0 comes along)

Since we allow future deprecation (and that shouldn't be affected
by 'no-deprecated'), we need to distinguish what to have deprecated
on the value of OPENSSL_API_COMPAT, not the existence of
OPENSSL_NO_DEPRECATED.

Note that the macro OPENSSL_NO_DEPRECATED still exists, in case
someone still uses it.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #6470)
levitte added a commit that referenced this pull request Jun 19, 2018
... to the check OPENSSL_API_COMPAT < 0x10100000L, to correspond with
how it's declared.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #6470)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants