Re: [2/2] dxdiagn: Successfully copy to destination variants with an invalid type in IDxDiagContainer::GetProp.

[Alexandre Julliard]

Andrew Nguyen  writes:
> @@ -221,9 +221,17 @@ static HRESULT WINAPI IDxDiagContainerImpl_GetProp(PDXDIAGCONTAINER iface, LPCWS
>
> p = This->properties;
>    while (NULL != p) {
> - if (0 == lstrcmpW(p->vName, pwszPropName)) {
> -
>  VariantCopy(pvarProp, &p->v);
> -
>  return S_OK;
> - if (0 == lstrcmpW(p->vName, pwszPropName)) {
> -
>  HRESULT hr = VariantClear(pvarProp);
> -
>  if (hr == DISP_E_ARRAYISLOCKED)
> -
>  {
> -
>    while (SafeArrayUnlock(V_ARRAY(pvarProp)) == S_OK);
> -
>    VariantClear(pvarProp);

This looks suspicious. Do you have evidence that the array should be
forcibly unlocked and destroyed?

[2/2] dxdiagn: Successfully copy to destination variants with an invalid type in IDxDiagContainer::GetProp.

[Andrew Nguyen]

On Tue, Mar 23, 2010 at 5:47 AM, Alexandre Julliard  wrote:
> Andrew Nguyen  writes:
> > @@ -221,9 +221,17 @@ static HRESULT WINAPI IDxDiagContainerImpl_GetProp(PDXDIAGCONTAINER iface, LPCWS
> >
> > p = This->properties;
> >    while (NULL != p) {
> > - if (0 == lstrcmpW(p->vName, pwszPropName)) {
> > - VariantCopy(pvarProp, &p->v);
> > - return S_OK;
> > - if (0 == lstrcmpW(p->vName, pwszPropName)) {
> > - HRESULT hr = VariantClear(pvarProp);
> > - if (hr == DISP_E_ARRAYISLOCKED)
> > - {
> > - while (SafeArrayUnlock(V_ARRAY(pvarProp)) == S_OK);
> > - VariantClear(pvarProp);
>
> This looks suspicious. Do you have evidence that the array should be
> forcibly unlocked and destroyed?
>
> --
> Alexandre Julliard
> [e-mail]

I thought I observed a crash on native when I tried to free the safe
array after the GetProp call, but some rechecking indicated a mistake
on my part. I'll resend a more reasonable patch.