Hi Shuai,
A few comments:
- We probably should propagate unrelated errors. So if FAILED(hres) &&
hres != DISP_E_TYPEMISMATCH, we should return hres
������ ������ if(FAILED(hres))
������ ������ ������ ������ return return_bool(res, FALSE);
������ ������ else
������ ������ ������ ������ return return_bool(res, TRUE);
Why not:
������ ������ return return_bool(res, SUCCEEDED(hres))
static HRESULT return_bool(VARIANT *res, BOOL val)
{
������ ������ if(res) {
������ ������ ������ ������ V_VT(res) = VT_BOOL;
������ ������ ������ ������ V_BOOL(res) = val;
������ ������ }
������ ������ return S_OK;
}
VARIANT_BOOL uses VARIANT_TRUE/VARIANT_FALSE values, so this should be:
V_BOOL(res) = val ? VARIANT_TRUE : VARIANT_FALSE;
Cheers,
Jacek
> <mailto:jacek@codeweavers.com>>:
On 07/18/14 17:29, Shuai Meng wrote:
> Thanks all for commenting, I changed the implementation and put the
> codes
> here: https://github.com/Shuai-Meng/wine/commit/bb212763ffd639a238b5a47f2c6a27047a79fdc5
>
> It passed on both winxp and wine. But this patch still needs your
> comments.
>
>
>
> 2014-07-18 17:30 GMT+08:00 Jacek Caban <jacek@codeweavers.com
>>> ������ ������ <mailto:dmitry@baikal.ru>>:
> ������ ������ On 07/18/14 04:11, Shuai Meng wrote:
>>
>>
>>
>> ������ ������ 2014-07-17 11:11 GMT+08:00 Dmitry Timoshkov <dmitry@baikal.ru
>>
>> ������ ������ ������ ������ Shuai Meng <mengshuaicalendr@gmail.com
>> ������ ������ <http://msdn.microsoft.com/en-us/library/9e7a57cf%28v=vs.84%29.aspx>>> ������ ������ ������ ������ <mailto:mengshuaicalendr@gmail.com>> wrote:
>>
>> ������ ������ ������ ������ > + ������ ������ ������ ������switch(V_VT(arg)) {
>> ������ ������ ������ ������ > + ������ ������ ������ ������case VT_UI1:
>> ������ ������ ������ ������ > + ������ ������ ������ ������case VT_I2:
>> ������ ������ ������ ������ > + ������ ������ ������ ������case VT_I4:
>> ������ ������ ������ ������ > + ������ ������ ������ ������case VT_I8:
>> ������ ������ ������ ������ > + ������ ������ ������ ������case VT_R4:
>> ������ ������ ������ ������ > + ������ ������ ������ ������case VT_R8:
>> ������ ������ ������ ������ > + ������ ������ ������ ������case VT_BOOL:
>> ������ ������ ������ ������ > + ������ ������ ������ ������case VT_EMPTY:
>> ������ ������ ������ ������ > + ������ ������ ������ ������case VT_CY:
>> ������ ������ ������ ������ > + ������ ������ ������ ������ ������ ������V_BOOL(res) = VARIANT_TRUE;
>> ������ ������ ������ ������ > + ������ ������ ������ ������ ������ ������break;
>>
>> ������ ������ ������ ������ This list is far from being complete. It seems that it should
>> ������ ������ ������ ������ also
>> ������ ������ ������ ������ contain at least VT_UI2, VT_UI4, VT_UI8, VT_I1, VT_INT, VT_UINT.
>> ������ ������ ������ ������ VT_VECTOR, VT_ARRAY and VT_BYREF modifers probably also should be
>> ������ ������ ������ ������ taken into account.
>>
>> ������ ������ ������ ������ --
>> ������ ������ ������ ������ Dmitry.
>>
>> ������ ������ Hi, I think over it again. Let's face the fact: what are all the
>> ������ ������ possible results of ������switch(V_VT(arg)) ? Will VT_UI2, VT_UI4,
>> ������ ������ VT_UI8, VT_I1, VT_INT, VT_UINT and so on exist? My answer is no.
>> ������ ������ arg comes from users' input, which means it only belongs to the
>> ������ ������ ������subtypes of variant, see here:
>> ������ ������ http://msdn.microsoft.com/en-us/library/9e7a57cf(v=vs.84).aspx
>
> ������ ������ No, your answer is wrong. VBScript code may be called from
> ������ ������ non-VBScript (and the other way around) and then any type of
> ������ ������ VARIANT may be passed around. Also, please add a test with an
> ������ ������ object having default value, which is of numeric type. I have a
> ������ ������ feeling that the right thing to do here is to call to_double and
> ������ ������ see if it fails or not, but this needs tests to confirm.
>
> ������ ������ Jacek
>
>