2014-04-21 17:18 GMT+08:00 Jacek Caban jacek@codeweavers.com:
Hi Shuai,
On 04/21/14 03:45, Shuai Meng wrote:
- return return_bool(res, val);
- V_VT(res) = VT_EMPTY;
- return VariantChangeType(res, arg, VARIANT_LOCALBOOL, VT_BOOL);
You can't assume that res is not NULL. If result of the function is not used, it will be NULL. A simple test case is like this:
call CBool(0)
Also, are you sure we want VARIANT_LOCALBOOL here? Did you test it (probably by running tests on localized Windows with translated string "False")?
I don't quite understand what this parameter means, in fact I just copy
it from to_string(). If not VARIANT_LOCALBOOL, waht esle should be filled here?
Looking at CBool tests:
+Call ok(CBool(Empty) = False, "CBool(Empty) = " & CBool(Empty)) +Call ok(getVT(CBool(Empty)) = "VT_BOOL", "getVT(CBool(Empty)) = " & getVT(CBool(Empty))) +Call ok(CBool(1) = True, "CBool(1) = " & CBool(1)) +Call ok(getVT(CBool(1)) = "VT_BOOL", "getVT(CBool(1)) = " & getVT(CBool(1))) +Call ok(CBool(CLng(0)) = False, "CBool(CLng(0)) = " & CBool(CLng(0))) +Call ok(getVT(CBool(CLng(0))) = "VT_BOOL", "getVT(CBool(CLng(0))) = " & getVT(CBool(CLng(0)))) +Call ok(CBool(CSng(0.5)) = True, "CBool(CSng(0.5)) = " & CBool(CSng(0.5))) +Call ok(getVT(CBool(CSng(0.5))) = "VT_BOOL", "getVT(CBool(CSng(0.5))) = " & getVT(CBool(CSng(0.5)))) +Call ok(CBool(-0.56) = True, "CBool(-0.56) = " & CBool(-0.56)) +Call ok(getVT(CBool(-0.56)) = "VT_BOOL", "getVT(CBool(-0.56)) = " & getVT(CBool(-0.56))) +Call ok(CBool(CCur(8.55)) = True, "CBool(CCur(8.55)) = " & CBool(CCur(8.55))) +Call ok(getVT(CBool(CCur(8.55))) = "VT_BOOL", "getVT(CBool(CCur(8.55))) = " & getVT(CBool(CCur(8.55)))) +Call ok(CBool(CDate(0)) = False, "CBool(CDate(0)) = " & CBool(CDate(0))) +Call ok(getVT(CBool(CDate(0))) = "VT_BOOL", "getVT(CBool(CDate(0))) = " & getVT(CBool(CDate(0)))) +Call ok(CBool(CDate(3.33)) = True, "CBool(CDate(3.33)) = " & CBool(CDate(3.33))) +Call ok(getVT(CBool(CDate(3.33))) = "VT_BOOL", "getVT(CBool(CDate(3.33))) = " & getVT(CBool(CDate(3.33)))) +Call ok(CBool("6666.78") = True, "CBool(""6666.78"") = " & CBool("6666.78")) +Call ok(getVT(CBool("6666.78")) = "VT_BOOL", "getVT(CBool(""6666.78"")) = " & getVT(CBool("6666.78"))) +Call ok(CBool(CByte(-0.1)) = False, "CBool(CByte(-0.1)) = " & CBool(CByte(-0.1))) +Call ok(getVT(CBool(CByte(-0.1))) = "VT_BOOL", "getVT(CBool(CByte(-0.1))) = " & getVT(CBool(CByte(-0.1))))
Those look mostly good. However I'd like to see some more tests. Interesting case is for example empty string and a "false", "False" and "#FALSE#" string. Also I'd really like to see tests on object default values. See the attached patch for an example.
Those comments apply also to other patches in the series. It may be easier to proceed if you resent just the first patch fixed and include tests that don't depending on other patches in the series in the patch itself.
Cheers, Jacek