[PATCH 1/3] dmloader: COM cleanup of IDirectMusicLoader object.
Christian Costa
titan.costa at gmail.com
Tue Nov 6 13:51:08 CST 2012
Le 06/11/2012 20:26, Nikolay Sivov a écrit :
> On 11/6/2012 20:47, Christian Costa wrote:
>> ---
>> dlls/dmloader/dmloader_private.h | 16 +++---
>> dlls/dmloader/loader.c | 107
>> +++++++++++++++++++++++---------------
>> 2 files changed, 73 insertions(+), 50 deletions(-)
>>
>> diff --git a/dlls/dmloader/dmloader_private.h
>> b/dlls/dmloader/dmloader_private.h
>> index 47a794c..913f3a6 100644
>> --- a/dlls/dmloader/dmloader_private.h
>> +++ b/dlls/dmloader/dmloader_private.h
>> @@ -91,14 +91,14 @@ typedef struct _WINE_LOADER_OPTION {
>> * IDirectMusicLoaderImpl implementation structure
>> */
>> struct IDirectMusicLoaderImpl {
>> - /* VTABLEs */
>> - const IDirectMusicLoader8Vtbl *LoaderVtbl;
>> - /* reference counter */
>> - LONG dwRef;
>> - /* simple cache (linked list) */
>> - struct list *pObjects;
>> - /* settings for certain object classes */
>> - struct list *pClassSettings;
>> + /* VTABLEs */
>> + IDirectMusicLoader8 IDirectMusicLoader8_iface;
>> + /* reference counter */
>> + LONG dwRef;
>> + /* simple cache (linked list) */
>> + struct list *pObjects;
>> + /* settings for certain object classes */
>> + struct list *pClassSettings;
>> };
> Please remove useless comments like VTABLES and reference counter and
> strip names from prefix, e.g. dwRef->ref.
It's done in next patch.
>> /* contained object entry */
>> diff --git a/dlls/dmloader/loader.c b/dlls/dmloader/loader.c
>> index cfb3de9..54108c7 100644
>> --- a/dlls/dmloader/loader.c
>> +++ b/dlls/dmloader/loader.c
>> @@ -21,6 +21,11 @@
>> WINE_DEFAULT_DEBUG_CHANNEL(dmloader);
>> +static inline IDirectMusicLoaderImpl*
>> impl_from_IDirectMusicLoader8(IDirectMusicLoader8 *iface)
>> +{
>> + return CONTAINING_RECORD(iface, IDirectMusicLoaderImpl,
>> IDirectMusicLoader8_iface);
>> +}
>> +
>> static HRESULT DMUSIC_InitLoaderSettings (LPDIRECTMUSICLOADER8 iface);
>> static HRESULT DMUSIC_GetLoaderSettings (LPDIRECTMUSICLOADER8
>> iface, REFGUID pClassID, WCHAR* wszSearchPath, LPBOOL pbCache);
>> static HRESULT DMUSIC_SetLoaderSettings (LPDIRECTMUSICLOADER8
>> iface, REFGUID pClassID, WCHAR* wszSearchPath, LPBOOL pbCache);
>> @@ -69,8 +74,9 @@ static BOOL DMUSIC_IsValidLoadableClass (REFCLSID
>> pClassID) {
>> */
>> /* IUnknown/IDirectMusicLoader(8) part: */
>> -static HRESULT WINAPI
>> IDirectMusicLoaderImpl_IDirectMusicLoader_QueryInterface
>> (LPDIRECTMUSICLOADER8 iface, REFIID riid, LPVOID *ppobj) {
>> - ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
>> +static HRESULT WINAPI
>> IDirectMusicLoaderImpl_IDirectMusicLoader_QueryInterface(LPDIRECTMUSICLOADER8
>> iface, REFIID riid, LPVOID *ppobj)
>> +{
>> + IDirectMusicLoaderImpl *This =
>> impl_from_IDirectMusicLoader8(iface);
>> TRACE("(%p, %s, %p)\n",This, debugstr_dmguid(riid), ppobj);
>> if (IsEqualIID (riid, &IID_IUnknown) ||
>> @@ -85,15 +91,17 @@ static HRESULT WINAPI
>> IDirectMusicLoaderImpl_IDirectMusicLoader_QueryInterface (
>> return E_NOINTERFACE;
>> }
>> -static ULONG WINAPI
>> IDirectMusicLoaderImpl_IDirectMusicLoader_AddRef
>> (LPDIRECTMUSICLOADER8 iface) {
>> - ICOM_THIS_MULTI(IDirectMusicLoaderImpl, LoaderVtbl, iface);
>> +static ULONG WINAPI
>> IDirectMusicLoaderImpl_IDirectMusicLoader_AddRef(LPDIRECTMUSICLOADER8
>> iface)
>> +{
>> + IDirectMusicLoaderImpl *This =
>> impl_from_IDirectMusicLoader8(iface);
>> TRACE("(%p): AddRef from %d\n", This, This->dwRef);
>> return InterlockedIncrement (&This->dwRef);
>> }
> What I also meant is these names are redundant, something like
> directmusicloader_AddRef() is enough, of course you could use mixed
> casing in names if you want, there's no strict guidelines for that afaik.
I like to see the interface name in the function name because the method
alone does not mean anything.
Something like this:
<object name>Impl_<interface name>_<method name>
but in some case where there is only 1 interface for the object:
<interface name>Impl_<method name>
is better.
The interface name should be clearly visible IMO it's easier to read in
a log file.
IDirectMusicLoaderImpl_IDirectMusicLoader_AddRef
And for example if an object has 2 interfaces X and Y that both derives
from Z then you will have 2 methods with the same name.
How will you make the distinction ?
If dmloader object has really 1 interface, I would rename then
IDirectMusicLoaderImpl_AddRef.
Although this is probably the case, I'm not sure yet. In all cases, I
would leave this for another patch. One thing at once.
More information about the wine-devel
mailing list