[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: jscript: Add VT_I2 support (try 2)
From:       Jacek Caban <jacek () codeweavers ! com>
Date:       2012-03-30 12:19:32
Message-ID: 4F75A4D4.8060407 () codeweavers ! com
[Download RAW message or body]

Hi Alistair,

On 03/30/12 12:29, Alistair Leslie-Hughes wrote:
> Hi,
> 
> 
> Changelog:
> jscript: Add VT_I2 support
> 

     hres = IDispatchEx_InvokeEx(dispex, id, ctx->lcid, flags, dp, retv, &ei->ei, \
&ctx->jscaller->IServiceProvider_iface); +    if(SUCCEEDED(hres) && retv)
+    {
+        if(V_VT(retv) == VT_I2)
+            VariantChangeType(retv, retv, 0, VT_I4);
+    }


Since the conversion here is trivial, why not do it directly?

--- a/dlls/jscript/jsutils.c
+++ b/dlls/jscript/jsutils.c
@@ -191,6 +191,10 @@ HRESULT to_primitive(script_ctx_t *ctx, VARIANT *v, jsexcept_t \
*ei, VARIANT *ret  case VT_R8:
         *ret = *v;
         break;
+    case VT_I2:
+        V_VT(ret) = VT_I4;
+        V_I4(ret) = V_I2(v);
+        break;


I thought the findings we've discussed off-list was that internal values
should never be of VT_I2 types, so this should never happen. If it's not
the case, I'd like to see a test case.

+function TestVT_I2()
+{
+    varI2 = GetShort();
+    num3 = 3
+
+    ok(getVT(varI2) === "VT_I4", "getVT(varI2) = " + getVT(varI2));
 

Please include getVT(GetShort()) test I've showed you. This eliminates a
lot of flow of value.

+    if(!strcmp_wa(bstrName, "GetShort")) {
+        *pid = DISPID_GLOBAL_SHORT;
+        return S_OK;
+    }


Please follow JavaScript names convention (this should be getShort).


Cheers,
Jacek


[prev in list] [next in list] [prev in thread] [next in thread]