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

winidn: drop WANT_IDN_PROTOTYPES #9793

Closed
wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Oct 24, 2022

WANT_IDN_PROTOTYPES was necessary to avoid using a header that
came via an optional package. MS stopped distributing this package some
years ago and the winidn definitions are part of standard headers (via
windows.h) since Vista.

Auto-detect Vista inside lib/idn_win32.c and enable the manual
definitions if building for an older Windows.

This allows to delete this manual knob from all build-systems.

Also drop the _SAL_VERSION sub-case:

The definitions are now only enabled with old systems. We assume that
code analysis is not run on such systems, allowing us to delete the
SAL-friendly flavour of these.

@vszakats vszakats added build Windows Windows-specific tidy-up labels Oct 24, 2022
@vszakats vszakats marked this pull request as draft October 24, 2022 09:28
@vszakats vszakats added the feature-window A merge of this requires an open feature window label Oct 24, 2022
@vszakats vszakats marked this pull request as ready for review October 25, 2022 15:23
Copy link
Member

@jay jay left a comment

Choose a reason for hiding this comment

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

The two commits should be combined. The first commit message's first sentence says:

This was necessary to avoid using a header that came via an optional package.

It would be clearer to say

WANT_IDN_PROTOTYPES was necessary to avoid using a header that came via an optional package.

because otherwise it could be read as "This (commit) was necessary to avoid using a header that came via an optional package."

`WANT_IDN_PROTOTYPES` was necessary to avoid using a header that came
via an optional package. MS stopped distributing this package some
years ago and the winidn definitions are part of standard headers (via
`windows.h`) since Vista.

Auto-detect Vista inside `lib/idn_win32.c` and enable the manual
definitions if building for an older Windows.

This allows to delete this manual knob from all build-systems.

Also drop the `_SAL_VERSION` sub-case:

The definitions are now only enabled with old systems. We assume that
code analysis is not run on such systems, allowing us to delete the
SAL-friendly flavour of these.
@vszakats
Copy link
Member Author

vszakats commented Oct 26, 2022

Thanks @jay, updated the text, squashed and rebased on master.

@vszakats vszakats removed the feature-window A merge of this requires an open feature window label Oct 26, 2022
@vszakats vszakats closed this in b51560b Oct 26, 2022
@vszakats vszakats deleted the win-idn-tidy-1 branch October 26, 2022 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants