Re: msi: Dont crash on installation of services without component KeyPath

[Aric Stewart]

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?