-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Correctly guard SetHandleInformation API call #1296
Conversation
`__cplusplus_winrt` only detected C++/CX (which can be used without compiling for UWP, SetHandleInformation would be available in those cases), and did not detect native UWP C++. This patch fixes that by using the WINAPI_FAMILY_PARTITION macro in the Windows SDK headers in the same way those headers remove SetHandleInformation in UWP builds.
How would you feel about adding a |
Which warnings specifically? The problem is spdlog is c++11 |
On second thought, What if windows sdk is not installed? will this break? |
Without the windows sdk, sethandleinformation or any windows header isn't
available at all. The warning is C4100, and maybe_unused could be
conditionally enabled using the preprocessor (or you could manually disable
the warning using pragmas)
…On Mon, Nov 4, 2019, 04:04 Gabi Melman ***@***.***> wrote:
On second thought.What if windows sdk is not installed? will this break?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1296?email_source=notifications&email_token=ABRELNTV74JU23SUTLT3HWLQR7QSPA5CNFSM4JIOWSQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC6STBA#issuecomment-549267844>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABRELNRVUIZPT4SGQMCVWGDQR7QSPANCNFSM4JIOWSQA>
.
|
Before which line? |
Are you sure? I dont see any mention of this here |
Around the method implementation's body since the warning is related to a
parameter not being used.
…On Mon, Nov 4, 2019, 08:55 Gabi Melman ***@***.***> wrote:
Without the windows sdk, sethandleinformation or any windows header isn't
available at all.
Are you sure? I dont see any mention of this here
<https://docs.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-sethandleinformation>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1296?email_source=notifications&email_token=ABRELNXO7Q7ZNJUN6KKDA4TQSASVDA5CNFSM4JIOWSQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7KA3A#issuecomment-549363820>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABRELNV3ILW54VTPHYYAI5TQSASVDANCNFSM4JIOWSQA>
.
|
You mean |
Correct, hence maybe_unused
…On Mon, Nov 4, 2019, 09:03 Gabi Melman ***@***.***> wrote:
You mean FILE *f not being used if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP
| WINAPI_PARTITION_SYSTEM is not satisfied ?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1296?email_source=notifications&email_token=ABRELNSB32B4Q6NW2ZLHHMTQSATSPA5CNFSM4JIOWSQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC7K3ZQ#issuecomment-549367270>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABRELNRVZDGVPBA5BLEXA53QSATSPANCNFSM4JIOWSQA>
.
|
The real problem is that this function would silently.. |
Here is my shot at it. If If |
Sounds like a valid option. |
|
__cplusplus_winrt
only detected C++/CX (which can be used without compiling for UWP, SetHandleInformation would be available in those cases), and did not detect native UWP C++. This patch fixes that by using the WINAPI_FAMILY_PARTITION macro in the Windows SDK headers in the same way those headers remove SetHandleInformation in UWP builds.