Re: [PATCH] vbscript: Implement StrComp()

[Jacek Caban]

Hi Nikolay,

The patch looks mostly good.


On 04.11.2016 07:34, Nikolay Sivov wrote:
> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>
> ---
>  dlls/vbscript/global.c      | 43 ++++++++++++++++++++++++++++++++++++++++---
>  dlls/vbscript/tests/api.vbs | 15 +++++++++++++++
>  2 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/vbscript/global.c b/dlls/vbscript/global.c
> index 129d6ce..a872838 100644
> --- a/dlls/vbscript/global.c
> +++ b/dlls/vbscript/global.c
> @@ -1027,10 +1027,47 @@ static HRESULT Global_MidB(vbdisp_t *This, VARIANT *arg, unsigned args_cnt, VARI
>      return E_NOTIMPL;
>  }
>
> -static HRESULT Global_StrComp(vbdisp_t *This, VARIANT *arg, unsigned args_cnt, VARIANT *res)
> +static HRESULT Global_StrComp(vbdisp_t *This, VARIANT *args, unsigned args_cnt, VARIANT *res)
>  {
> -    FIXME("\n");
> -    return E_NOTIMPL;
> +    BSTR left, right;
> +    int mode, ret;
> +    HRESULT hres;
> +
> +    TRACE("(%s %s ...)\n", debugstr_variant(args), debugstr_variant(args+1));
> +
> +    assert(args_cnt == 2 || args_cnt == 3);
> +
> +    if(V_VT(args) != VT_BSTR || V_VT(args+1) != VT_BSTR) {
> +        FIXME("args[0] = %s, args[1] = %s\n", debugstr_variant(args), debugstr_variant(args+1));
> +        return E_NOTIMPL;
> +    }
> +
> +    if (args_cnt == 3) {
> +        hres = to_int(args+2, &mode);

A test where mode arg is of other type would be nice. Something like "1"
(passed as a string).
> +        if(FAILED(hres))
> +            return hres;
> +
> +        if (mode != 0 && mode != 1) {
> +            FIXME("unknown compare mode = %d\n", mode);
> +            return E_FAIL;
> +        }
> +    }
> +    else
> +        mode = 0;
> +
> +    left = V_BSTR(args);
> +    right = V_BSTR(args+1);
> +
> +    V_VT(res) = VT_I2;
> +    ret = mode ? strcmpiW(left, right) : strcmpW(left, right);
> +    if (ret < 0)
> +        V_I2(res) = -1;
> +    else if (ret > 0)
> +        V_I2(res) = 1;
> +    else
> +        V_I2(res) = 0;

Please use return_short() helper here.

Thanks,
Jacek