setupapi: use INF parser to read class GUID and class name
Previously, SetupDiGetINFClassW()
read INF files
with GetPrivateProfileString()
, which does not substitute %strkey%
tokens.
This caused device installation to fail for devices which had driver INF files using %strkey% tokens in Version section.
An example of such device is Vernier LabQuest Mini (08f7:0008) for which Vernier's LoggerPro application includes a driver.
The INF file in question adds a new device setup class and has following entries in Version section:
Class = %ClassName%
ClassGuid = %DeviceClassGUID%
Strings section includes following entries:
DeviceClassGUID = "{6B8429BF-10AD-4b66-9FBA-2FE72B891721}"
ClassName = "VST_WinUSB"
Previously, when LoggerPro was installed and LabQuest Mini was hotplugged, device installation failed with the following error:
fixme:setupapi:SetupDiGetINFClassW failed to convert "L"%DeviceClassGUID"" into a guid
This caused GUID_NULL to be used and Class was not set to the registry for the device.
With this commit, correct class GUID and names are set to the device registry entry.
Merge request reports
Activity
Sure! There was an existing test case for reading INF class, so I just added couple of tests to the end. Interestingly, the test code used profile API to write to the INF file, so I just decided to go with the flow and used
WritePrivateProfileStringA()
too.So, now there are tests, which use profile API to write to an INF file, to ensure the code it is testing does not use profile API to read from an INF file. How ironic! But I think that's fine, given that all we need from testing point of view is the ability to write variable definitions to sections.
requested review from @zfigura
added 4 commits
-
18262972...166895ae - 2 commits from branch
wine:master
- a9a72b33 - setupapi/tests: add tests for reading INF class with %strkey% tokens
- 72ac1a4b - setupapi: use INF parser to read class GUID and class name
-
18262972...166895ae - 2 commits from branch
- Resolved by Elizabeth Figura
@zfigura My end goal is to make Vernier's LabQuest Mini work with Wine. I don't know how much work it requires, or if it's even possible, in a practical sense. I'm not (yet) experienced with Wine codebase (nor with Win API), but by looking at setupapi, pnp, etc., I get a feeling that it should be possible.
My current understanding, based on log messages, is that the main reason why it's not already working is that
load_function_driver()
cannot find the driver (the driver does not get installed correctly) on the device startup. And that there are few obstacles before that, the bug this MR tries to solve being the first one.So, what do you think? Is there hope and if yes, what would be the most obvious things I should be looking at next?
Oh, I see, now I know.
But I guess it's still a good idea to introduce tests in a separate commit? At least the removal of
wine_todo
s gives good indication what the commit is aiming for..Now the first commit brings in tests with some parts annotated with
todo_wine
and the second commit removes annotations.But I guess it's still a good idea to introduce tests in a separate commit? At least the removal of
wine_todo
s gives good indication what the commit is aiming for..Yes, precisely. When it's feasible that helps a reviewer tell what the commit is trying to do. It also validates that it really is solving the problem it's supposed to, assuming the tests pass as they're supposed to.
2014 2016 todo_wine 2015 2017 ok(count == 4, "expected count==4, got %lu(%s)\n", count, cn); 2016 2018 2019 /* Test Strings substitution */ 2020 WritePrivateProfileStringA("Version", "Class", "%ClassName%", filename); 2021 WritePrivateProfileStringA("Version", "ClassGUID", "%ClassGuid%", filename); 2022 2023 /* Without Strings section the ClassGUID is invalid */ 2024 retval = SetupDiGetINFClassA(filename, &guid, cn, MAX_PATH, NULL); 2025 ok(!retval, "expected SetupDiGetINFClassA to fail because ClassGUID is invalid (has non-substituted strkey token)!\n"); changed this line in version 4 of the diff
4351 have_name = SetupFindFirstLineW(hinf, Version, Class, &inf_ctx); 4327 4352 4328 if (dret >= MAX_PATH -1) FIXME("buffer might be too small\n"); 4353 class_name_len = 0; 4354 if (have_name) 4355 { 4356 if (!SetupGetStringFieldW(&inf_ctx, 1, buffer, ARRAY_SIZE(buffer), NULL)) 4357 { 4358 ERR("failed to get [Version].Class as a string from '%s'\n", debugstr_w(inf)); 4359 goto out; 4360 } 4361 4362 class_name_len = lstrlenW(buffer); 4363 } 4364 4365 if (class_name_len >= MAX_PATH - 1) FIXME("buffer might be too small\n"); This conditional FIXME was there already, so no change here (other than the local variable rename).
But now that I just wrote that, I think I got what you mean and what this conditional FIXME was used for previously.
So this FIXME was never about detecting overruns, because the overrun was impossible also previously. But it's point is to issue a warning that
MAX_PATH
might not be enough to capture the whole value of the key, i.e. if there was a risk to get a truncated value; if the buffer gets completely filled, it's possible that part of the value got left out.But now,
SetupGetStringFieldW
parses the whole value and checks if the buffer is actually big enough to store it, so indeed, this FIXME is completely redundant.changed this line in version 4 of the diff
added 66 commits
-
26238664...7f190e67 - 64 commits from branch
wine:master
- d8bbf6d0 - setupapi/tests: Add tests for reading INF class with %strkey% tokens.
- 987695a4 - setupapi: Use INF parser to read class GUID and class name.
-
26238664...7f190e67 - 64 commits from branch