Re: ADVAPI32: Start test for service tests

2007-06-05 Thread Alexandre Julliard
"Rolf Kalbermatter" <[EMAIL PROTECTED]> writes:

> Go figure. To me it seems they messed up with the conversion from Widechar to
> ASCII when needing to calculate the needed size. Unless they use some other
> function than WideCharToMultiByte() it couldn't be for avoiding an extra 
> buffer
> allocation since WideCharToMultiByte() specifically states that the input and
> output buffer can not be the same pointer. Maybe that native NTDLL provides a
> lower level Unicode -> ASCII conversion that does allow for overlapping
> buffers.

No, returning twice the W size is the normal behavior when you don't
want to retrieve the full W text but only its length. Since each W
char can map to at most two A chars (at least with standard Windows
codepages) doubling the size is an upper bound on the A buffer size,
and doing it that way can avoid a lot of unnecessary work.

-- 
Alexandre Julliard
[EMAIL PROTECTED]




RE: ADVAPI32: Start test for service tests

2007-06-04 Thread Rolf Kalbermatter
James Hawkins [mailto:[EMAIL PROTECTED] wrote:

>...and msdn is wrong a lot, but that doesn't mean the check is right.
>If the value you get is 1, then check exactly for 1.

I can currently only check on XP and the behaviour I see there does seem
strange to me. It should rather return with some error and not change the
size at all or maybe it just returns the default key, which usually should
be an empty string. The test as is was meant to have at least some starting
point. There will be many additions and possibly changes in the future as
other results get known from different Windows versions.

>Huh?  The function, A or W, returns the number of TCHARS.
>What behavior should wine not imitate?  We don't ignore expected behavior
>just because we doubt an app will depend on it, especiallly a behavior that
>is so common.

That is what it says in MSDN yes. But the reality is different!

GetServiceDisplayNameA(hsvcm, "eventlog", NULL, &size);

returns with 19 for the size but returns then only a string "Event Log".

GetServiceDisplayNameW(hsvcm, L"eventlog", NULL, &size);

returns with 9 for the size and returns with the string "Event Log".

Go figure. To me it seems they messed up with the conversion from Widechar
to
ASCII when needing to calculate the needed size. Unless they use some other
function than WideCharToMultiByte() it couldn't be for avoiding an extra
buffer
allocation since WideCharToMultiByte() specifically states that the input
and
output buffer can not be the same pointer. Maybe that native NTDLL provides
a
lower level Unicode -> ASCII conversion that does allow for overlapping
buffers.
Then this strange behaviour could make sense when implementing the A
function
on top of the W function as it would avoid one temporary buffer allocation
for
the conversion. Our implementation currently directly accesses the according
registry API to read the string so the actual Unicode conversion happens in
the registry API.

While we certainly could hack our ASCII fucntion to behave exactly as the
Windows one I do not see any reason to do so. For any properly implemented
application it will cause unnecessary memory allocations only and for the
others it won't make any difference. 

And all this also leaves the problem of localization using different strings
for the display name so checking for an explicit string length simply won't
work, which was your main criticisme here in the beginning.

Rolf Kalbermatter





Re: ADVAPI32: Start test for service tests

2007-05-21 Thread James Hawkins

On 5/21/07, Rolf Kalbermatter <[EMAIL PROTECTED]> wrote:

James Hawkins [mailto:[EMAIL PROTECTED] wrote:

>+ok(size <= 1, "size should be <= 1 was %d!", size);
>
>This is a bad test.  According to msdn, GetServiceDisplayName does not
>modify lpcchBuffer on error, so size should be exactly 0.

MSDN says a lot of things. On my XP SP2 it returns 1 as size when passed
an empty service name. Also failing here is a bit ambigeuos since the
dunction also fails when a NULL pointer is passed or the buffer is to
small, but it does change the size into what it expects in these two
cases.



...and msdn is wrong a lot, but that doesn't mean the check is right.
If the value you get is 1, then check exactly for 1.


>+static BOOL test_service_info(SC_HANDLE hscm)
>
>test functions are void, not BOOL.  You're never going to do anything but
>return, and there's no value >to check by the caller.

I took this more or less directly from registry.c in the same directory.



No test function in registry.c returns BOOL, and to save you some time
replying, set_privileges is not a test function.  It's a helper
function.


>+ok(size, "size should be returned!");
>
>And what is the size?  Tests are supposed to be exact.  If our
implementation
>is incorrect, and we return size as 39491, that's certainly not correct,
but
>this test won't fail when it should.

This won't work for localized Windows versions. The display name is possibly
translated. Also, on XP SP2 the A function returns the size in ASCI chars
that
the W function will need in bytes. Not a behaviour I feel Wine should
imitate
without an app that would need that behaviour.



Huh?  The function, A or W, returns the number of TCHARS.  What
behavior should wine not imitate?  We don't ignore expected behavior
just because we doubt an app will depend on it, especiallly a behavior
that is so common.


I'll be gone for two weeks for vacation. If anyone wants to continue on that
please feel free. Otherwise I'll be picking this up afterwards.

Rolf Kalbermatter





--
James Hawkins




RE: ADVAPI32: Start test for service tests

2007-05-21 Thread Rolf Kalbermatter
James Hawkins [mailto:[EMAIL PROTECTED] wrote:

>+ok(size <= 1, "size should be <= 1 was %d!", size);
>
>This is a bad test.  According to msdn, GetServiceDisplayName does not
>modify lpcchBuffer on error, so size should be exactly 0.

MSDN says a lot of things. On my XP SP2 it returns 1 as size when passed
an empty service name. Also failing here is a bit ambigeuos since the
dunction also fails when a NULL pointer is passed or the buffer is to
small, but it does change the size into what it expects in these two
cases.

>+static BOOL test_service_info(SC_HANDLE hscm)
>
>test functions are void, not BOOL.  You're never going to do anything but
>return, and there's no value >to check by the caller.

I took this more or less directly from registry.c in the same directory.

>+ok(size, "size should be returned!");
>
>And what is the size?  Tests are supposed to be exact.  If our
implementation
>is incorrect, and we return size as 39491, that's certainly not correct,
but
>this test won't fail when it should.

This won't work for localized Windows versions. The display name is possibly
translated. Also, on XP SP2 the A function returns the size in ASCI chars
that
the W function will need in bytes. Not a behaviour I feel Wine should
imitate
without an app that would need that behaviour.

I'll be gone for two weeks for vacation. If anyone wants to continue on that
please feel free. Otherwise I'll be picking this up afterwards.

Rolf Kalbermatter





Re: ADVAPI32: Start test for service tests

2007-05-20 Thread James Hawkins

On 5/20/07, Rolf Kalbermatter <[EMAIL PROTECTED]> wrote:

Changelog
  dlls/advapi32/tests/Makefile.in
  dlls/advapi32/tests/service.c
Start test for service tests
License: X11/LGPL

This test will crash without the previous patch
"Return error on NULL service name to GetServiceDisplayNameA/W"

Rolf Kalbermatter



+ok(size <= 1, "size should be <= 1 was %d!", size);

This is a bad test.  According to msdn, GetServiceDisplayName does not
modify lpcchBuffer on error, so size should be exactly 0.

+static BOOL test_service_info(SC_HANDLE hscm)

test functions are void, not BOOL.  You're never going to do anything
but return, and there's no value to check by the caller.

+ok(size, "size should be returned!");

And what is the size?  Tests are supposed to be exact.  If our
implementation is incorrect, and we return size as 39491, that's
certainly not correct, but this test won't fail when it should.

+static BOOL test_service_enum(SC_HANDLE hscm)
+{
+return 0;
+}

Please don't add dead code.

--
James Hawkins