Re: msi: Dont crash on installation of services without component KeyPath
On Wed, 2014-02-19 at 14:49 +0900, Aric Stewart wrote:
diff --git a/dlls/msi/action.c b/dlls/msi/action.c index 07322fe..a004225 100644 --- a/dlls/msi/action.c +++ b/dlls/msi/action.c @@ -5794,27 +5794,30 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param) goto done; } key = MSI_RecordGetString(row, 6); - file = msi_get_loaded_file(package, key); - msiobj_release(&row->hdr); - if (!file) - { - ERR("Failed to load the service file\n"); - goto done; - } - - if (!args || !args[0]) image_path = file->TargetPath; - else + if (key) { - int len = strlenW(file->TargetPath) + strlenW(args) + 2; - if (!(image_path = msi_alloc(len * sizeof(WCHAR)))) + file = msi_get_loaded_file(package, key); + msiobj_release(&row->hdr); + if (!file) { - ret = ERROR_OUTOFMEMORY; + ERR("Failed to load the service file\n"); goto done; }
- strcpyW(image_path, file->TargetPath); - strcatW(image_path, szSpace); - strcatW(image_path, args); + if (!args || !args[0]) image_path = file->TargetPath; + else + { + int len = strlenW(file->TargetPath) + strlenW(args) + 2; + if (!(image_path = msi_alloc(len * sizeof(WCHAR)))) + { + ret = ERROR_OUTOFMEMORY; + goto done; + } + + strcpyW(image_path, file->TargetPath); + strcatW(image_path, szSpace); + strcatW(image_path, args); + } } service = CreateServiceW(hscm, name, disp, GENERIC_ALL, serv_type, start_type, err_control, image_path, load_order, @@ -5831,7 +5834,7 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param) WARN("failed to set service description %u\n", GetLastError()); }
- if (image_path != file->TargetPath) msi_free(image_path); + if (image_path && image_path != file->TargetPath) msi_free(image_path); done: CloseServiceHandle(service); CloseServiceHandle(hscm);
Should we still create the service if the installer doesn't specify the image path?
Well the image path in the component is optional and looking at the documentation for CreateServiceW lpBinaryPathName is also optional. So it seemed correct. If you feel like we should just exit then that is acceptable as well. -aric On 2014/02/19 17:51, Hans Leidekker wrote:
On Wed, 2014-02-19 at 14:49 +0900, Aric Stewart wrote:
diff --git a/dlls/msi/action.c b/dlls/msi/action.c index 07322fe..a004225 100644 --- a/dlls/msi/action.c +++ b/dlls/msi/action.c @@ -5794,27 +5794,30 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param) goto done; } key = MSI_RecordGetString(row, 6); - file = msi_get_loaded_file(package, key); - msiobj_release(&row->hdr); - if (!file) - { - ERR("Failed to load the service file\n"); - goto done; - } - - if (!args || !args[0]) image_path = file->TargetPath; - else + if (key) { - int len = strlenW(file->TargetPath) + strlenW(args) + 2; - if (!(image_path = msi_alloc(len * sizeof(WCHAR)))) + file = msi_get_loaded_file(package, key); + msiobj_release(&row->hdr); + if (!file) { - ret = ERROR_OUTOFMEMORY; + ERR("Failed to load the service file\n"); goto done; }
- strcpyW(image_path, file->TargetPath); - strcatW(image_path, szSpace); - strcatW(image_path, args); + if (!args || !args[0]) image_path = file->TargetPath; + else + { + int len = strlenW(file->TargetPath) + strlenW(args) + 2; + if (!(image_path = msi_alloc(len * sizeof(WCHAR)))) + { + ret = ERROR_OUTOFMEMORY; + goto done; + } + + strcpyW(image_path, file->TargetPath); + strcatW(image_path, szSpace); + strcatW(image_path, args); + } } service = CreateServiceW(hscm, name, disp, GENERIC_ALL, serv_type, start_type, err_control, image_path, load_order, @@ -5831,7 +5834,7 @@ static UINT ITERATE_InstallService(MSIRECORD *rec, LPVOID param) WARN("failed to set service description %u\n", GetLastError()); }
- if (image_path != file->TargetPath) msi_free(image_path); + if (image_path && image_path != file->TargetPath) msi_free(image_path); done: CloseServiceHandle(service); CloseServiceHandle(hscm);
Should we still create the service if the installer doesn't specify the image path?
On Wed, 2014-02-19 at 17:57 +0900, Aric Stewart wrote:
Well the image path in the component is optional and looking at the documentation for CreateServiceW lpBinaryPathName is also optional. So it seemed correct.
If you feel like we should just exit then that is acceptable as well.
What happens on Windows?
I have not dug into the details, but I know it does not crash and the overall install does succeed. I can try investigating further on windows, Is there an easy way to determine what service this is and if it gets successfully installed on windows? -aric On 2014/02/19 18:11, Hans Leidekker wrote:
On Wed, 2014-02-19 at 17:57 +0900, Aric Stewart wrote:
Well the image path in the component is optional and looking at the documentation for CreateServiceW lpBinaryPathName is also optional. So it seemed correct.
If you feel like we should just exit then that is acceptable as well.
What happens on Windows?
On Wed, 2014-02-19 at 20:03 +0900, Aric Stewart wrote:
I have not dug into the details, but I know it does not crash and the overall install does succeed.
I can try investigating further on windows, Is there an easy way to determine what service this is and if it gets successfully installed on windows?
A +msi,+service trace should reveal the service name. "sc query" will list installed services, for example.
Ok, I will try that tomorrow and report if windows successfully installs the service or not. -aric On 2014/02/19 20:28, Hans Leidekker wrote:
On Wed, 2014-02-19 at 20:03 +0900, Aric Stewart wrote:
I have not dug into the details, but I know it does not crash and the overall install does succeed.
I can try investigating further on windows, Is there an easy way to determine what service this is and if it gets successfully installed on windows?
A +msi,+service trace should reveal the service name. "sc query" will list installed services, for example.
On 2014/02/19 20:28, Hans Leidekker wrote:
On Wed, 2014-02-19 at 20:03 +0900, Aric Stewart wrote:
I have not dug into the details, but I know it does not crash and the overall install does succeed.
I can try investigating further on windows, Is there an easy way to determine what service this is and if it gets successfully installed on windows?
A +msi,+service trace should reveal the service name. "sc query" will list installed services, for example.
Unfortunately it only tries to install the service on windows 2000 or older. (this is an old applications that I was installing in a windows 2000 prefix) I do not have a windows 2000 machine to test the install on. Using an XP prefix gets around the problem by not trying to install the service. But I am unable to determine what is the correct method to fix this issue. You are more of the expert Hans, so I will leave it up to you if we should fail to install the service or install the service without an image path. -aric
On Thu, 20 Feb 2014, Aric Stewart wrote: [...]
Unfortunately it only tries to install the service on windows 2000 or older. (this is an old applications that I was installing in a windows 2000 prefix) I do not have a windows 2000 machine to test the install on.
You're in luck then. The new testbot now has a Windows 2000 VM... -- Francois Gouget <fgouget(a)codeweavers.com>
Francois Gouget <fgouget(a)codeweavers.com> wrote:
You're in luck then. The new testbot now has a Windows 2000 VM...
Probably it's time then to make testbot process the tasks slightly faster, currently it's speed is completely unacceptable. For that reason I was using old testbot instead, but since it's no longer available this creates additional problems and prevents productive work. -- Dmitry.
participants (4)
-
Aric Stewart -
Dmitry Timoshkov -
Francois Gouget -
Hans Leidekker