ole32: Accept STG_E_UNIMPLEMENTEDFUNCTION when ILockBytes implementation doesn't support locking.

[Dmitry Timoshkov]

Nikolay Sivov <bunglehead at gmail.com> wrote:

> >>>        /* If the ILockBytes doesn't support locking that's ok. */
> >>> -    if (hr == STG_E_INVALIDFUNCTION) return S_OK;
> >>> +    if (hr == STG_E_INVALIDFUNCTION || hr == STG_E_UNIMPLEMENTEDFUNCTION) return S_OK;
> >>>        else if (FAILED(hr)) return hr;
> >>>
> >>>        hr = S_OK;
> >>>
> >>
> >> It looks like StorageImpl_LockRegionSync should be fixed instead to
> >> filter some error codes. Could you add a test with external ILockBytes
> >> implementation that does that?
> >
> > Thers is no any comment or a test case about accepting ILockByte failures
> > neither in the original commit 65887802c502c4eeeb3fc905990e3e2f4548a482,
> > nor in the next one 1645f7b9e30e01bc92a8bfd1febe76d4856e046e which limits
> > accepted error codes to STG_E_INVALIDFUNCTION.
> 
> Well it doesn't matter, we don't have tests for many things, it doesn't 
> mean we shouldn't ever think about having one.

Thinking is good I guess.

> > Besides, my patch fixes a regression caused by IStorage locking rewrite,
> > and should be pretty obvious without a test case. With this regression
> > fixed an application that I have here can open its database files again
> > as it was before.
> >
> > If you really insist on a test case, please ask Vincent, he is the original
> > authour of this implementation.
> >
> 
> He's not the one sending this, so why should he be writing tests for 
> your changes?

My patch just adds another obvious case of a possible ILockBytes failure.
If the tests were not needed for an already existing case then my patch
changes nothing in that regard. Fixing a regression with a pretty obvious
fix shouldn't deserve so much of salt from your side Nikolay.

-- 
Dmitry.