Re: ws2_32: Fix conversion of address in GetAddrInfoW

[Dmitry Timoshkov]

Bruno Jesus  wrote:
> -
>    if (a->ai_canonname && b->ai_canonname)
> -
>    {
> -
>        char bufa[128], bufb[128];
> -
>        WideCharToMultiByte(CP_ACP, 0, a->ai_canonname, -1,
> -
>                            bufa, sizeof(bufa), NULL, NULL);
> -
>        WideCharToMultiByte(CP_ACP, 0, b->ai_canonname, -1,
> -
>                            bufb, sizeof(bufb), NULL, NULL);
> -
>        ok(!strcmp(bufa, bufb), "Wrong canonical name '%s' != '%s'\n", bufa, bufb);
> -
>    }

I'd suggest to avoid using fixed size buffers and call lstrcmpW() for
comparison and wine_dbgstr_w() to print the values.
> -
>    else if(a->ai_canonname || b->ai_canonname)
> -
>        ok(0, "Expected both canonical names present (%p != %p)\n", a->ai_canonname, b->ai_canonname);

It would look more natural IMHO to use
ok(!a->ai_canonname && !b->ai_canonname, "Expected both canonical names present (%p != %p)\n", a->ai_canonname, b->ai_canonname);
instead of explicit ok(0) (here and in the not unicode variant).
> - if (a || b)
> -
>    ok(0, "Expected both addresses null (%p != %p)\n", a, b);

Same here.

ws2_32: Fix conversion of address in GetAddrInfoW

[Bruno Jesus]

On Tue, Feb 18, 2014 at 12:11 AM, Dmitry Timoshkov  wrote:
> Bruno Jesus  wrote:
> > -
> >    if (a->ai_canonname && b->ai_canonname)
> > -
> >    {
> > -
> >        char bufa[128], bufb[128];
> > -
> >        WideCharToMultiByte(CP_ACP, 0, a->ai_canonname, -1,
> > -
> >                            bufa, sizeof(bufa), NULL, NULL);
> > -
> >        WideCharToMultiByte(CP_ACP, 0, b->ai_canonname, -1,
> > -
> >                            bufb, sizeof(bufb), NULL, NULL);
> > -
> >        ok(!strcmp(bufa, bufb), "Wrong canonical name '%s' != '%s'\n", bufa, bufb);
> > -
> >    }
>
> I'd suggest to avoid using fixed size buffers and call lstrcmpW() for
> comparison and wine_dbgstr_w() to print the values.

For some stupid reason I was trying to use wine_debugstr_w instead of
wine_dbgstr_w, so I looked for an alternative way...
> > -
> >    else if(a->ai_canonname || b->ai_canonname)
> > -
> >        ok(0, "Expected both canonical names present (%p != %p)\n", a->ai_canonname, b->ai_canonname);
>
> It would look more natural IMHO to use
> ok(!a->ai_canonname && !b->ai_canonname, "Expected both canonical names present (%p != %p)\n", a->ai_canonname, b->ai_canonname);
> instead of explicit ok(0) (here and in the not unicode variant).

Indeed, much better.
> > - if (a || b)
> > -
> >    ok(0, "Expected both addresses null (%p != %p)\n", a, b);
>
> Same here.

Yep.
> --
> Dmitry.

Thanks, Dmitry.

Bruno

ws2_32: Fix conversion of address in GetAddrInfoW

[Dmitry Timoshkov]

Bruno Jesus  wrote:
> > > -
> > >    else if(a->ai_canonname || b->ai_canonname)
> > > -
> > >        ok(0, "Expected both canonical names present (%p != %p)\n", a->ai_canonname, b->ai_canonname);
> >
> > It would look more natural IMHO to use
> > ok(!a->ai_canonname && !b->ai_canonname, "Expected both canonical names present (%p != %p)\n", a->ai_canonname, b->ai_canonname);
> > instead of explicit ok(0) (here and in the not unicode variant).
>
> Indeed, much better.

Please take into account my typos above (remove '!'s).

ws2_32: Fix conversion of address in GetAddrInfoW

[Bruno Jesus]

On Tue, Feb 18, 2014 at 1:06 AM, Dmitry Timoshkov  wrote:
> Bruno Jesus  wrote:
> > > > -
> > > >    else if(a->ai_canonname || b->ai_canonname)
> > > > -
> > > >        ok(0, "Expected both canonical names present (%p != %p)\n", a->ai_canonname, b->ai_canonname);
> > >
> > > It would look more natural IMHO to use
> > > ok(!a->ai_canonname && !b->ai_canonname, "Expected both canonical names present (%p != %p)\n", a->ai_canonname, b->ai_canonname);
> > > instead of explicit ok(0) (here and in the not unicode variant).
> >
> > Indeed, much better.
>
> Please take into account my typos above (remove '!'s).

If one of the strings are not present both must be null. So it's OK to
have ok(!a->ai_canonname && !b->ai_canonname). Actually I changed the
text to reflect that:

if (a->ai_canonname && b->ai_canonname)
         {
             ok(!strcmp(a->ai_canonname, b->ai_canonname), "Wrong
canonical name '%s' != '%s'\n",
                a->ai_canonname, b->ai_canonname);
         }
         else
             ok(!a->ai_canonname && !b->ai_canonname, "Expected both
names absent (%p != %p)\n",
                  a->ai_canonname, b->ai_canonname);

I think that it's correct like this.
> --
> Dmitry.