Hello Joerg,
and addendum inline
Michael Stefaniuc wrote:
> joerg-cyril.hoe...@t-systems.com wrote:
>> Michael Stefaniuc wrote:
>>> static HRESULT WINAPI IDirectSoundCaptureImpl_CreateCaptureBuffer
>> + if (pUnk) {
>> + WARN("invalid parameter: pUnk != NULL\n");
>> + /* *lplpDSCaptureBuffer = NULL; Not done by native dsound */
>> + return DSERR_NOAGGREGATION;
>> + }
>>
>> What's your rationale for doing so?
>> Bug compatibility with native is fine, however not setting NULL is
>> problematic before you've reached 100% compatibility, have you?
> Not in this case which deals with COM aggregation. dsound doesn't
> supports that and it is documented.
>
>> What I mean is that if there's a situation where Wine's CreateCaptureBuffer
>> returns some DSERR such as NOTIMPL where native does not, then that's an
>> unexpected situation for the app which it may not be well prepared to handle,
>> as it is untestable on native. Now not zeroing output pointers may make it
>> even harder for the app to remain in sane state.
>> Then blame the app or Wine for a crash?
>>
>> If OTOH you know that the Wine module *always* returns the exact same
>> values as native, then I've no objection in not zeroing output pointers when
>> native does neither. Yet you'll have to consider that native may do so in
>> the future, witness a growing number of MSDN documents covering later
>> APIs that specifically mention zeroing pointers in case of failure.
>>
>> IIRC, AJ removed from my mmdevapi tests one snippet that showed that
>> native's GetService
>> does not zero an output pointer even though MSDN says so -- Wine zeroes. Yet
>> perhaps
>> I got confused, messed up with patches and submitted the one that did not
>> contain that test
>> so maybe AJ never had a chance to see it. I'll have to check that.
> Patch 8/8 has the tests for this. I used the standard COM aggregation
> test for classes that do not support COM aggregation. That was designed
> by Jacek in
> http://source.winehq.org/git/wine.git/commitdiff/5b16e6e0fdb35e57a034fd5e6df61765ad11fa71
>
> And the
> ok(!unk, "unk = %p\n", unk);
Although I personally didn't hit that case yet my guess that check isn't
there just to detect the implementation details of the error handling.
It is there to detect if a class pretends to not support COM aggregation
but which still supports it. Just to give their own applications a
"competitive advantage" ...
> failed on the Win7 box I used for testing. That test is in because it is
> known that Windows has classes that do not follow their own COM
> standard, especially when it comes to COM aggregation.
>
> Now i would have had different options how to deal with that.
> - Silently drop the test.
> - Dropping the test and documenting why it deviates from the standard test.
> - Adding the test and marking it todo_wine.
> - Add the test and fix the code as the change is trivial.
>
> I picked IMHO the cleanest solution as that matches native 100% and adds
> a test for that behavior.
bye
michael
