Vitaly Lipatov wrote:
В сообщении от 25 декабря 2008 Вы написали: ...
You could start with testing for some special cases (such as NULL input parameters).
If I will add test case for NULL input and returned values, my commit will be accepted?
I don't know actually. That's first I think about when my patch isn't accepted.
For me your comment looks suspicious:
- TODO
- Check if OLE object has no verbs
-----
- hr = IOleObject_EnumVerbs(lpOleObj, &pEnumVerbs);
- if (hr == S_OK) {
Will this hr be OLEOBJ_E_NOVERBS when no verbs present? If it's so you're actually checking for this. This one needs a testcase for example.
Please understand me. I did a part of work. Possible my implementation is incomplete. But I wish to share result of my work with the world, winehq community. I agree with test case argument with I fix already existing function. But I do a new realization and have no enough time and knowledge to write ideal code. Also I can improve code only from winehq repository. ...
All of this is perfectly clear, and I don't argue with you and I'm not calling your patch bad. I'm only point on some obvious things for me. Treat this only like an advice and my opinion.
Does native crash on it or not? I'm not sure.
The same about SetLastError on failure - this should be checked by testing it directly.
I will try to add this tests.