Re: [PATCH] dmime: Move finding a track to a help function

[Michael Stefaniuc]

Hello AListair,

any reason for doing this? E.g. do you need the helper somewhere else?

I'm normally doing the exact opposite and move the implementation from
single use helpers to the method itself.

bye
    michael

On 3/24/20 8:53 AM, Alistair Leslie-Hughes wrote:
> Signed-off-by: Alistair Leslie-Hughes
>
> dlls/dmime/segment.c | 98 ++++++++++++++++++++++++++------------------
>  1 file changed, 57 insertions(+), 41 deletions(-)
>
> diff --git a/dlls/dmime/segment.c b/dlls/dmime/segment.c
> index 0455b6c5ddf..cfc67a7757d 100644
> --- a/dlls/dmime/segment.c
> +++ b/dlls/dmime/segment.c
> @@ -160,53 +160,69 @@ static HRESULT WINAPI IDirectMusicSegment8Impl_SetDefaultResolution(IDirectMusic
>    return S_OK;
>  }
>
> +static LPDMUS_PRIVATE_SEGMENT_TRACK get_track_entry(IDirectMusicSegment8Impl *This, REFGUID type,
> -
>            DWORD group, DWORD index)
>
> +{
> - struct list* pEntry = NULL;
> - LPDMUS_PRIVATE_SEGMENT_TRACK pIt = NULL;
> - CLSID pIt_clsid;
> - IPersistStream* pCLSIDStream = NULL;
> -
> - LIST_FOR_EACH (pEntry, &This->Tracks) {
> -
>    pIt = LIST_ENTRY(pEntry, DMUS_PRIVATE_SEGMENT_TRACK, entry);
> -
>    TRACE(" - %p -> 0x%x,%p\n", pIt, pIt->dwGroupBits, pIt->pTrack);
> -
>    if (0xFFFFFFFF != group && 0 == (pIt->dwGroupBits & group)) continue;
> -
>    if (!IsEqualGUID(&GUID_NULL, type)) {
> -
>        HRESULT hr;
> -
>        /**
> -
>         * it rguidType is not null we must check if CLSIDs are equal
> -
>         * and the unique way to get it is using IPersistStream Interface
> -
>         */
> -
>        hr = IDirectMusicTrack_QueryInterface(pIt->pTrack, &IID_IPersistStream, (void**) &pCLSIDStream);
> -
>        if (FAILED(hr)) {
> -
>            ERR("(%p): object %p don't implement IPersistStream Interface. Expect a crash (critical problem)\n",
> -
>                    This, pIt->pTrack);
> -
>            continue;
> -
>        }
> -
>        hr = IPersistStream_GetClassID(pCLSIDStream, &pIt_clsid);
> -
>        IPersistStream_Release(pCLSIDStream); pCLSIDStream = NULL;
> -
>        if (FAILED(hr)) {
> -
>            ERR("(%p): non-implemented GetClassID for object %p\n", This, pIt->pTrack);
> -
>            continue;
> -
>        }
> -
>        TRACE(" - %p -> %s\n", pIt, debugstr_dmguid(&pIt_clsid));
> -
>        if (!IsEqualGUID(&pIt_clsid, type)) continue;
> -
>    }
> -
>    if (0 == index) {
> -
>        return pIt;
> -
>    }
> -
>    --index;
> - }
> -
> - return NULL;
>
> +}
> -
>
> static HRESULT WINAPI IDirectMusicSegment8Impl_GetTrack(IDirectMusicSegment8 *iface,
> -
>    REFGUID rguidType, DWORD dwGroupBits, DWORD dwIndex, IDirectMusicTrack **ppTrack)
> -
>    REFGUID type, DWORD group, DWORD index, IDirectMusicTrack **track)
>
> {
> - IDirectMusicSegment8Impl *This = impl_from_IDirectMusicSegment8(iface);
> - CLSID pIt_clsid;
> - struct list* pEntry = NULL;
> - LPDMUS_PRIVATE_SEGMENT_TRACK pIt = NULL;
> - IPersistStream* pCLSIDStream = NULL;
> - HRESULT hr = S_OK;
> - IDirectMusicSegment8Impl *This = impl_from_IDirectMusicSegment8(iface);
> - LPDMUS_PRIVATE_SEGMENT_TRACK item = NULL;
> - TRACE("(%p, %s, %#x, %#x, %p)\n", This, debugstr_dmguid(rguidType), dwGroupBits, dwIndex, ppTrack);
> - TRACE("(%p, %s, %#x, %#x, %p)\n", This, debugstr_dmguid(type), group, index, track);
> - if (NULL == ppTrack) {
> - return E_POINTER;
> - }
> - if (!track) {
> -
>    return E_POINTER;
> - }
> - LIST_FOR_EACH (pEntry, &This->Tracks) {
> - pIt = LIST_ENTRY(pEntry, DMUS_PRIVATE_SEGMENT_TRACK, entry);
> - TRACE(" - %p -> 0x%x,%p\n", pIt, pIt->dwGroupBits, pIt->pTrack);
> - if (0xFFFFFFFF != dwGroupBits && 0 == (pIt->dwGroupBits & dwGroupBits)) continue ;
> - if (FALSE == IsEqualGUID(&GUID_NULL, rguidType)) {
> -
>  /**
> -
>   * it rguidType is not null we must check if CLSIDs are equal
> -
>   * and the unique way to get it is using IPersistStream Interface
> -
>   */
> -
>  hr = IDirectMusicTrack_QueryInterface(pIt->pTrack, &IID_IPersistStream, (void**) &pCLSIDStream);
> -
>  if (FAILED(hr)) {
> - ERR("(%p): object %p don't implement IPersistStream Interface. Expect a crash (critical problem)\n", This, pIt->pTrack);
> - continue ;
> -
>  }
> -
>  hr = IPersistStream_GetClassID(pCLSIDStream, &pIt_clsid);
> -
>  IPersistStream_Release(pCLSIDStream); pCLSIDStream = NULL;
> -
>  if (FAILED(hr)) {
> - ERR("(%p): non-implemented GetClassID for object %p\n", This, pIt->pTrack);
> - continue ;
> -
>  }
> -
>  TRACE(" - %p -> %s\n", pIt, debugstr_dmguid(&pIt_clsid));
> -
>  if (FALSE == IsEqualGUID(&pIt_clsid, rguidType)) continue ;
> - item = get_track_entry(This, type, group, index);
> - if (item)
> - {
> -
>      *track = item->pTrack;
> -
>      IDirectMusicTrack_AddRef(*track);
> -
>      return S_OK;
> }
> - if (0 == dwIndex) {
> -
>  *ppTrack = pIt->pTrack;
> -
>  IDirectMusicTrack_AddRef(*ppTrack);
> -
>  return S_OK;
> - }
> - --dwIndex;
> - }
> - return DMUS_E_NOT_FOUND;
> -
> - return DMUS_E_NOT_FOUND;
>
> }
>
> static HRESULT WINAPI IDirectMusicSegment8Impl_GetTrackGroup(IDirectMusicSegment8 *iface,