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

remove WDK dependency for OpenCL ICD loader #102

Merged
merged 3 commits into from
Apr 23, 2020

Conversation

bashbaug
Copy link
Contributor

@bashbaug bashbaug commented Apr 1, 2020

This is a similar change as #90, but it goes a bit farther and removes all WDK dependencies, including header file dependencies. Instead of including a WDK header file, a private header file is used, which contains just the structure and function definitions needed by the OpenCL ICD loader. This is the same mechanism used by the Vulkan loader, and indeed the private header file is copied unchanged from the Vulkan loader repo - see https://github.com/KhronosGroup/Vulkan-Loader/blob/master/loader/adapters.h.

I've intentionally kept the code flow fairly unchanged, and most of the diffs are due to slightly different structure member names in adapters.h. At some point it would be beneficial to refactor this function slightly, I think, but that can be done as a follow-on PR.

The one change I did include in this PR is to use the "secure" version of wcstombs, so I believe this PR fixes #93.

@bashbaug
Copy link
Contributor Author

bashbaug commented Apr 1, 2020

Ping @nikhiljnv - I can't request a review from you for some reason, but I would appreciate if you would take a look at these changes. Thanks!

@nikhiljnv
Copy link
Contributor

Ping @nikhiljnv - I can't request a review from you for some reason, but I would appreciate if you would take a look at these changes. Thanks!

Thanks Ben for taking time from your busy schedule. Sorry, this just fell off my radar.
Change looks good to me.

@bashbaug
Copy link
Contributor Author

Change looks good to me.

Thanks Nikhil.

@b-sumner @saleelk, if there are no concerns from your side, I will plan to merge these changes right after #103.

After this change, instead of including a WDK header file, a
private header file is used with just the definitions needed
for the ICD loader.  The header file is copied unchanged from
the Vulkan loader repo.
@bashbaug
Copy link
Contributor Author

Merging after @nikhiljnv 's +1 above.

@bashbaug bashbaug merged commit fe09ad1 into KhronosGroup:master Apr 23, 2020
joekilner pushed a commit to joekilner/OpenCL-ICD-Loader that referenced this pull request Jun 20, 2020
* remove dependency on Windows WDK for Windows ICD Loader builds

After this change, instead of including a WDK header file, a
private header file is used with just the definitions needed
for the ICD loader.  The header file is copied unchanged from
the Vulkan loader repo.

* update README to remove WDK dependency

* updated file version to v2.2.8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'wcstombs': This function or variable may be unsafe
2 participants