Re: jscript: Add VT_I2 support (try 2)

[Jacek Caban]

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