Re: [PATCH 4/4] vbscript: Implemented Abs(try 2)

[Jacek Caban]

On 06/17/14 08:00, Shuai Meng wrote:
> diff --git a/dlls/vbscript/global.c b/dlls/vbscript/global.c
> index d21784d..e646a29 100644
> --- a/dlls/vbscript/global.c
> +++ b/dlls/vbscript/global.c
> @@ -1302,8 +1302,24 @@ static HRESULT Global_ChrW(vbdisp_t *This, VARIANT *arg, unsigned args_cnt, VARI
>
> static HRESULT Global_Abs(vbdisp_t *This, VARIANT *arg, unsigned args_cnt, VARIANT *res)
>  {
> - FIXME("\n");
> - return E_NOTIMPL;
> - VARIANT v;
> - HRESULT hres;
> -
> - TRACE("(%s)\n", debugstr_variant(arg));
> -
> - assert(args_cnt == 1);
> -
> - V_VT(&v) = VT_EMPTY;
> - hres = VariantChangeType(&v, arg, VARIANT_LOCALBOOL, VT_R8);
> -
>    if (FAILED(hres))
> -
>        return S_OK;

Please use to_double here. Also note weird indention here.

Jacek

[PATCH 4/4] vbscript: Implemented Abs(try 2)

[Shuai Meng]

Hi,
I notice that Abs(Null) should return Null according to
http://msdn.microsoft.com/en-us/library/307330xe(v=vs.84).aspx

In wine source code, NULL is defined as 0 in
file dlls/windowscodecs/ungif.h, line 66. I have tried to write like this:
1260     if(V_VT(arg) == VT_NULL) {
1261         V_VT(res) = VT_NULL;
1262         V_NULL(res) = NULL;
1263     }
1264     else {
1265         hres = to_double(arg, &v);
1266         if (FAILED(hres))
1267             return hres;
1268
1269         if(res) {
1270             V_VT(res) = VT_R8;
1271             V_R8(res) = v >= 0 ? v : -v;
1272         }
1273     }
1274     return S_OK;

But apparently this makes no sense: V_NULL is defined
in dlls/oleaut32/tests/vartest.c, not along with V_I2 in include/oleauto.h;
I don't know how give res a "Null" value in C.

2014-06-17 17:44 GMT+08:00 Jacek Caban :
> On 06/17/14 08:00, Shuai Meng wrote:
> > diff --git a/dlls/vbscript/global.c b/dlls/vbscript/global.c
> > index d21784d..e646a29 100644
> > --- a/dlls/vbscript/global.c
> > +++ b/dlls/vbscript/global.c
> > @@ -1302,8 +1302,24 @@ static HRESULT Global_ChrW(vbdisp_t *This,
>
> VARIANT *arg, unsigned args_cnt, VARI
> > static HRESULT Global_Abs(vbdisp_t *This, VARIANT *arg, unsigned
>
> args_cnt, VARIANT *res)
> > {
> > - FIXME("\n");
> > - return E_NOTIMPL;
> > - VARIANT v;
> > - HRESULT hres;
> > -
> > - TRACE("(%s)\n", debugstr_variant(arg));
> > -
> > - assert(args_cnt == 1);
> > -
> > - V_VT(&v) = VT_EMPTY;
> > - hres = VariantChangeType(&v, arg, VARIANT_LOCALBOOL, VT_R8);
> > -
> >    if (FAILED(hres))
> > -
> >        return S_OK;
>
> Please use to_double here. Also note weird indention here.
>
> Jacek

[PATCH 4/4] vbscript: Implemented Abs(try 2)

[Piotr Caban]

On 06/26/14 06:05, Shuai Meng wrote:
> Hi,
> I notice that Abs(Null) should return Null according to
> http://msdn.microsoft.com/en-us/library/307330xe(v=vs.84).aspx
>
> In wine source code, NULL is defined as 0 in
> file dlls/windowscodecs/ungif.h, line 66. I have tried to write like this:
> 1260     if(V_VT(arg) == VT_NULL) {
> 1261         V_VT(res) = VT_NULL;
> 1262         V_NULL(res) = NULL;
> 1263     }

In case of VT_NULL variant type there's no need to set variant value. 
You only need to set V_VT(res) to VT_NULL. You also need to handle 
res==NULL case here.

[PATCH 4/4] vbscript: Implemented Abs(try 2)

[Shuai Meng]

2014-06-26 16:08 GMT+08:00 Piotr Caban :
> On 06/26/14 06:05, Shuai Meng wrote:
> > Hi,
> > I notice that Abs(Null) should return Null according to
> > http://msdn.microsoft.com/en-us/library/307330xe(v=vs.84).aspx
> >
> > In wine source code, NULL is defined as 0 in
> > file dlls/windowscodecs/ungif.h, line 66. I have tried to write like this:
> > 1260     if(V_VT(arg) == VT_NULL) {
> > 1261         V_VT(res) = VT_NULL;
> > 1262         V_NULL(res) = NULL;
> > 1263     }
>
> In case of VT_NULL variant type there's no need to set variant value. You
> only need to set V_VT(res) to VT_NULL.

Thank you Piotr. I get it.
> You also need to handle res==NULL case here.

I guess you mean that I need to handle res==NULL case in branch
if(V_VT(arg) == VT_NULL), right?

There is another problem: though Abs(Null) should return Null according to
http://msdn.microsoft.com/en-us/library/307330xe(v=vs.84).aspx , but I test
it in WINXP, i.e.  when arg == Null, the vbscript code:
Call Abs(Null)
and
dim b
b = Abs(Null)
MsgBox b
 returns an error.

[PATCH 4/4] vbscript: Implemented Abs(try 2)

[Piotr Caban]

On 06/26/14 12:13, Shuai Meng wrote:
> I guess you mean that I need to handle res==NULL case in branch
> if(V_VT(arg) == VT_NULL), right?

Yes, but return_null() function will already cover it for you.
> There is another problem: though Abs(Null) should return Null according to
> http://msdn.microsoft.com/en-__us/library/307330xe(v=vs.84).__aspx
> http://msdn.microsoft.com/en-us/library/307330xe(v=vs.84).aspx , but I
> test it in WINXP, i.e.  when arg == Null, the vbscript code:
> Call Abs(Null)
> and
> dim b
> b = Abs(Null)
> MsgBox b
>   returns an error.

I don't see this issue. Probably you have done something incorrectly. 
Here's result from a testbot run that tests Abs(Null). The tests are 
working on all machines.
http://newtestbot.winehq.org/JobDetails.pl?Key=7712

[PATCH 4/4] vbscript: Implemented Abs(try 2)

[Jacek Caban]

On 06/26/14 06:05, Shuai Meng wrote:
> In wine source code, NULL is defined as 0 in
> file dlls/windowscodecs/ungif.h, line 66. I have tried to write like this:
> 1260     if(V_VT(arg) == VT_NULL) {
> 1261         V_VT(res) = VT_NULL;
> 1262         V_NULL(res) = NULL;
> 1263     }

Hint: there already is return_null helper.

Jacek

[PATCH 4/4] vbscript: Implemented Abs(try 2)

[Shuai Meng]

2014-06-26 16:20 GMT+08:00 Jacek Caban :
> On 06/26/14 06:05, Shuai Meng wrote:
> > In wine source code, NULL is defined as 0 in
> > file dlls/windowscodecs/ungif.h, line 66. I have tried to write like
>
> this:
> > 1260     if(V_VT(arg) == VT_NULL) {
> > 1261         V_VT(res) = VT_NULL;
> > 1262         V_NULL(res) = NULL;
> > 1263     }
>
> Hint: there already is return_null helper.
>
> Jacek

Thank you Jacek, that helps a lot :)  I should read source code more
frequently in case of forgetting something.