Re: [PATCH 3/3] vbscript: Fixed CInt

[Nikolay Sivov]

> - hres = to_int(arg, &val);
> - V_VT(&v) = VT_EMPTY;
> - hres = VariantChangeType(&v, arg, 0, VT_I2);
>  if(FAILED(hres))
>      return hres;
> - return return_int(res, val);
> - if(!res) {
> -
>    VariantClear(&v);
> -
>    return DISP_E_BADVARTYPE;
> - }
> -
> - *res = v;
> - return S_OK;

I can't comment on removing to_int() here, I don't know this code well. 
But is it really possible to get a NULL 'res'? It looks like you can 
just use 'res' without intermediate 'v'. In any case you don't need 
VariantClear() on VT_I2 variant.

[PATCH 3/3] vbscript: Fixed CInt

[Piotr Caban]

On 5/23/14 6:36 AM, Nikolay Sivov wrote:
> > - hres = to_int(arg, &val);
> > - V_VT(&v) = VT_EMPTY;
> > - hres = VariantChangeType(&v, arg, 0, VT_I2);
> >  if(FAILED(hres))
> >      return hres;
> > - return return_int(res, val);
> > - if(!res) {
> > -
> >    VariantClear(&v);
> > -
> >    return DISP_E_BADVARTYPE;
> > - }
> > -
> > - *res = v;
> > - return S_OK;
>
> I can't comment on removing to_int() here, I don't know this code 
> well. But is it really possible to get a NULL 'res'? It looks like you 
> can just use 'res' without intermediate 'v'. In any case you don't 
> need VariantClear() on VT_I2 variant.

VariantChangeType is handling more variant types then to_int. There are 
tests that show that e.g. VT_DISPATCH and VT_BSTR needs to be handled.

It's possible that res is NULL, there are tests for it (it happens when 
result of CInt is not used, e.g. Call CInt(0)).

He can't use 'res' without intermediate 'v'. It's needed to return the 
same error code as native does. There are also tests for it.

I don't have strong opinion about calling VariantClear but I think it's 
good to call it.