Re: msi: PublishComponents and MsiGetQualifiedComponentExW

[Mike McCormack]

Aric Stewart wrote:
> - WCHAR buf[21];
> - WCHAR buf2[21];

It would be nicer to get rid of the magic numbers, and to call the 
variables something more meaningful than buf1, buf2.
> -
>
> rc = MSI_DatabaseOpenViewW(package->db, ExecSeqQuery, &view);
> -
>
> if (rc != ERROR_SUCCESS)
> -
>    return ERROR_SUCCESS;
> -
> -
>
> rc = MSI_IterateRecords(view, NULL, ITERATE_PublishComponent, package);
>
> return rc;
>
> }

You need to release the view you opened.  MSI_IterateRecords leaves the 
view intact (it does not call msiobj_release).
> @@ -1297,11 +1298,93 @@
>                  DWORD Unused1, DWORD Unused2, LPWSTR lpPathBuf,
>                  DWORD* pcchPathBuf)
>  {
> - FIXME("%s %s %li %s %li %li %p %p\n", debugstr_w(szComponent),
> - HKEY hkey;
> - UINT rc;
> - WCHAR info[124];

What is info, and why is it 124 bytes long?
> - DWORD sz;
> - LPWSTR product = NULL;
> - LPWSTR component = NULL;
> - LPWSTR ptr;
> - GUID clsid;
> -
> - TRACE("%s %s %li %s %li %li %p %p\n", debugstr_w(szComponent),
>       debugstr_w(szQualifier), dwInstallMode, debugstr_w(szProduct),
>       Unused1, Unused2, lpPathBuf, pcchPathBuf);
> -
> - rc = MSIREG_OpenUserComponentsKey(szComponent, &hkey, FALSE);
> - if (rc != ERROR_SUCCESS)
> -
>    return ERROR_INDEX_ABSENT;
> -
> - sz = sizeof(info);
> - rc = RegQueryValueExW( hkey, szQualifier, NULL, NULL, (LPVOID) info, &sz);
> - if (rc != ERROR_SUCCESS)
> -
>    return ERROR_INDEX_ABSENT;

You need to close the hkey you opened.
> - rc = MsiProvideQualifiedComponentW(szwComponent, szwQualifier,
> -
>                dwInstallMode, lpwPathBuf, &pcchwPathBuf);
> -
> - HeapFree(GetProcessHeap(),0,szwComponent);
> - HeapFree(GetProcessHeap(),0,szwQualifier);
> - WideCharToMultiByte(CP_ACP, 0, lpwPathBuf, pcchwPathBuf, lpPathBuf,
> -
>                *pcchPathBuf, NULL, NULL);
> - *pcchPathBuf = pcchwPathBuf;

This is wrong for multibyte locales, isn't it?
> Index: dlls/msi/msipriv.h
>
> RCS file: /home/wine/wine/dlls/msi/msipriv.h,v
> retrieving revision 1.52
> diff -u -r1.52 msipriv.h
> --- dlls/msi/msipriv.h	11 Apr 2005 12:47:20 -0000	1.52
> +++ dlls/msi/msipriv.h	18 Apr 2005 18:57:52 -0000
> @@ -357,6 +357,7 @@
>  extern UINT MSIREG_OpenFeatures(HKEY* key);
>  extern UINT MSIREG_OpenFeaturesKey(LPCWSTR szProduct, HKEY* key, BOOL create);
>  extern UINT MSIREG_OpenComponents(HKEY* key);
> +extern UINT MSIREG_OpenUserComponentsKey(LPCWSTR szComponent, HKEY* key, BOOL create);
>  extern UINT MSIREG_OpenComponentsKey(LPCWSTR szComponent, HKEY* key, BOOL create);
>  extern UINT MSIREG_OpenProductsKey(LPCWSTR szProduct, HKEY* key, BOOL create);
>  extern UINT MSIREG_OpenUserFeaturesKey(LPCWSTR szProduct, HKEY* key, BOOL create);

Can't we have one or two functions that do the same thing as all the 
above to avoid the duplication of similar code?

Mike

msi: PublishComponents and MsiGetQualifiedComponentExW

[Aric Stewart]

Thanks for the pointers, I have resubmitted the patch taking these into 
account.
> > Index: dlls/msi/msipriv.h
> >
> > RCS file: /home/wine/wine/dlls/msi/msipriv.h,v
> > retrieving revision 1.52
> > diff -u -r1.52 msipriv.h
> > --- dlls/msi/msipriv.h    11 Apr 2005 12:47:20 -0000    1.52
> > +++ dlls/msi/msipriv.h    18 Apr 2005 18:57:52 -0000
> > @@ -357,6 +357,7 @@
> >  extern UINT MSIREG_OpenFeatures(HKEY* key);
> >  extern UINT MSIREG_OpenFeaturesKey(LPCWSTR szProduct, HKEY* key, 
> > BOOL create);
> >  extern UINT MSIREG_OpenComponents(HKEY* key);
> > +extern UINT MSIREG_OpenUserComponentsKey(LPCWSTR szComponent, HKEY* 
> > key, BOOL create);
> >  extern UINT MSIREG_OpenComponentsKey(LPCWSTR szComponent, HKEY* key, 
> > BOOL create);
> >  extern UINT MSIREG_OpenProductsKey(LPCWSTR szProduct, HKEY* key, 
> > BOOL create);
> >  extern UINT MSIREG_OpenUserFeaturesKey(LPCWSTR szProduct, HKEY* key, 
> > BOOL create);
>
> Can't we have one or two functions that do the same thing as all the 
> above to avoid the duplication of similar code?

These functions are pretty small, and there is not really any 
substantial duplicated code. I think it is just a coding preference. My 
natural tendency is to make a larger range of smaller functions with 
fewer parameters instead of one more general function with obscure 
parameters and added complexity.

-aric