Re: [5/5] ws2_32/tests: Tests for invalid conditions in InetNtop

[Paul Vriens]

Jeff Latimer wrote:
> MSDN defines an number of conditions that inet_ntop will return errors. 
>  This patch tests for them.
>
> dlls/ws2_32/socket.c     |   30 +++++++++++++++-
>  dlls/ws2_32/tests/sock.c |   83 
> ++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 109 insertions(+), 4 deletions(-)

I think you can get rid of these comments:

/* MSDN says it returns ERROR_INVALID_PARAMETER in MSDN but Vista returns
    0xc000000d - STATUS_INVALID_PARAMETER */

MSDN has proven to be wrong on occasion and there is no need to put that in the 
code.

Secondly you shouldn't use magic number if you no the name:

WSASetLastError(0xc000000d);

In the tests you do:

+        ok(WSAGetLastError() == 0xC000000D,
+           "Should be ERROR_INVALID_PARAMETER not %x\n", WSAGetLastError());

Again the magic number and in this case the ok() message is wrong as that should 
be STATUS_INVALID_PARAMETER according to your testing.

[5/5] ws2_32/tests: Tests for invalid conditions in InetNtop

[Jeff Latimer]

Paul Vriens wrote:
> Jeff Latimer wrote:
> > MSDN defines an number of conditions that inet_ntop will return 
> > errors.  This patch tests for them.
> > -
>
> Secondly you shouldn't use magic number if you no the name:
>
> WSASetLastError(0xc000000d);
>
> In the tests you do:
> -
>    ok(WSAGetLastError() == 0xC000000D,
> -
>       "Should be ERROR_INVALID_PARAMETER not %x\n", 
>
> WSAGetLastError());
>
> Again the magic number and in this case the ok() message is wrong as 
> that should be STATUS_INVALID_PARAMETER according to your testing.

STATUS_INVALID_PARAMETER is defined in ntstatus.h and I did not think 
that ntstatus.h was appropriate in ws2_32.  What do you think?  I can 
put in easily enough.

[5/5] ws2_32/tests: Tests for invalid conditions in InetNtop

[Paul Vriens]

Jeff Latimer wrote:
> Paul Vriens wrote:
> > Jeff Latimer wrote:
> > > MSDN defines an number of conditions that inet_ntop will return 
> > > errors.  This patch tests for them.
> > > -
> >
> > Secondly you shouldn't use magic number if you no the name:
> >
> > WSASetLastError(0xc000000d);
> >
> > In the tests you do:
> > -
> >    ok(WSAGetLastError() == 0xC000000D,
> > -
> >       "Should be ERROR_INVALID_PARAMETER not %x\n", 
> >
> > WSAGetLastError());
> >
> > Again the magic number and in this case the ok() message is wrong as 
> > that should be STATUS_INVALID_PARAMETER according to your testing.
>
> STATUS_INVALID_PARAMETER is defined in ntstatus.h and I did not think 
> that ntstatus.h was appropriate in ws2_32.  What do you think?  I can 
> put in easily enough.

I don't know of any side effects for including so I'd say add it. We have more 
tests that include ntstatus.h.

[5/5] ws2_32/tests: Tests for invalid conditions in InetNtop

[Paul Vriens]

Paul Vriens wrote:
> Jeff Latimer wrote:
> > Paul Vriens wrote:
> > > Jeff Latimer wrote:
> > > > MSDN defines an number of conditions that inet_ntop will return 
> > > > errors.  This patch tests for them.
> > > > -
> > >
> > > Secondly you shouldn't use magic number if you no the name:
> > >
> > > WSASetLastError(0xc000000d);
> > >
> > > In the tests you do:
> > > -
> > >    ok(WSAGetLastError() == 0xC000000D,
> > > -
> > >       "Should be ERROR_INVALID_PARAMETER not %x\n", 
> > >
> > > WSAGetLastError());
> > >
> > > Again the magic number and in this case the ok() message is wrong as 
> > > that should be STATUS_INVALID_PARAMETER according to your testing.
> >
> > STATUS_INVALID_PARAMETER is defined in ntstatus.h and I did not think 
> > that ntstatus.h was appropriate in ws2_32.  What do you think?  I can 
> > put in easily enough.
>
> I don't know of any side effects for including so I'd say add it. We 
> have more tests that include ntstatus.h.

Oh and ws2_32/socket.c also includes ntstatus.h