Re: [oleaut 3/3] oleaut32: Implement IPropertyBag::Read proxying

[Rob Shearman]

2009/1/1 Michael Karcher <wine(a)mkarcher.dialup.fu-berlin.de>:
> +  hr = IPropertyBag_RemoteRead_Proxy(This, pszPropName, &outVariant, pErrorLog,
> +                                       V_VT(pVar), pUnk);
> +  if(SUCCEEDED(hr))
> +    hr = VariantCopy(pVar, &outVariant);
> +
> +  return hr;

You're leaking the memory in outVariant here, since VariantCopy does a
deep copy of all of the data. Is there a reason you don't just pass
pVar into IPropertyBag_RemoteRead_Proxy?

-- 
Rob Shearman

[oleaut 3/3] oleaut32: Implement IPropertyBag::Read proxying

[Michael Karcher]

Am Donnerstag, den 01.01.2009, 18:58 +0000 schrieb Rob Shearman:
> 2009/1/1 Michael Karcher <wine(a)mkarcher.dialup.fu-berlin.de>:
> > +  hr = IPropertyBag_RemoteRead_Proxy(This, pszPropName, &outVariant, pErrorLog,
> > +                                       V_VT(pVar), pUnk);
> > +  if(SUCCEEDED(hr))
> > +    hr = VariantCopy(pVar, &outVariant);
> > +
> > +  return hr;
>
> You're leaking the memory in outVariant here, since VariantCopy does a
> deep copy of all of the data. Is there a reason you don't just pass
> pVar into IPropertyBag_RemoteRead_Proxy?
> Yes. I want outVariant to be unmodified in the case of error. But that
> might be wrong. Even if I pass pVar into IPropertyBag_RemoteRead_Proxy,
> i have to take care of leaking, I think, as RemoteRead takes the variant
> pointer as an out-only parameter, so I would have to VariantClear
> outVariant before. But you are right that this might be the better
> solution.

Thanks for reviewing my patches.

Regards,
  Michael Karcher

[oleaut 3/3] oleaut32: Implement IPropertyBag::Read proxying

[Rob Shearman]

2009/1/1 Michael Karcher <wine(a)mkarcher.dialup.fu-berlin.de>:
> Am Donnerstag, den 01.01.2009, 18:58 +0000 schrieb Rob Shearman:
> > 2009/1/1 Michael Karcher <wine(a)mkarcher.dialup.fu-berlin.de>:
> > > +  hr = IPropertyBag_RemoteRead_Proxy(This, pszPropName, &outVariant, pErrorLog,
> > > +                                       V_VT(pVar), pUnk);
> > > +  if(SUCCEEDED(hr))
> > > +    hr = VariantCopy(pVar, &outVariant);
> > > +
> > > +  return hr;
> >
> > You're leaking the memory in outVariant here, since VariantCopy does a
> > deep copy of all of the data. Is there a reason you don't just pass
> > pVar into IPropertyBag_RemoteRead_Proxy?
> > Yes. I want outVariant to be unmodified in the case of error. But that
> > might be wrong. Even if I pass pVar into IPropertyBag_RemoteRead_Proxy,
> > i have to take care of leaking, I think, as RemoteRead takes the variant
> > pointer as an out-only parameter, so I would have to VariantClear
> > outVariant before. But you are right that this might be the better
> > solution.

According to NDR marshalling rules, even if a parameter is marked as
[out]-only memory is still re-used so you don't have to clear the
VARIANT before passing it into IPropertyBag_RemoteRead_Proxy. For this
reason, if you don't decide to pass pVar straight into
IPropertyBag_RemoteRead_Proxy you still have to do VariantInit on
outVariant before passing it into IPropertyBag_RemoteRead_Proxy.

-- 
Rob Shearman