On 18.08.2016 15:27, Aric Stewart wrote:
Thanks for the review, made most of these changes. One question.
On 8/18/16 4:35 AM, Sebastian Lackner wrote:
driver = load_device_driver(szService, szMatch);
if (!driver)
ERR("failed to load driver %s\n", debugstr_w(szService));
StartServiceW(service, 0, NULL);
CloseServiceHandle(service);
The registry parsing code is probably fine, but wouldn't it be easier to handle this similar to other autostart services in services.exe? Then there would also be no need to spawn a separate thread for initialization.
I don't get what you mean. Instead of using StartServiceW duplicating the code from services.exe? I don't feel like that is the correct plan. Part of the reason for doing all the work in services.exe and winedevice.exe was to prevent code duplication and to be able to do that StartServiceW instead. If I was going to duplicate code I would duplicate winedevice.exe and just do it that way instead.
-aric
Sorry if I was a bit unclear. My idea was not to duplicate code from services.exe into your driver, but instead to merge your "autostart" code into services.exe. You would have to read the database twice then (one time in services.exe to add autostart services, and a second time in wineplugplay.sys), but a big advantage would be that you can remove the separate thread in wineplugplay.sys. Also, by starting all services from one single place it would be easier to ensure that they are loaded in the correct order. Wouldn't it have advantages to implement the startup like that, or am I missing something?
BTW: If you have to keep StartServiceW in wineplugplay.sys, it would be good to check the error code and repeat the call if the database is still locked from the regular process startup (ERROR_SERVICE_DATABASE_LOCKED).
Regards, Sebastian