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 Oct 24, 2022
@vszakats vszakats marked this pull request as ready for review Oct 25, 2022
jay
jay approved these changes Oct 26, 2022
Copy link
Member

@jay jay left a comment

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.

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

Successfully merging this pull request may close these issues.

None yet

2 participants