Re: [Setuapi] implement SetupDiCreateDeviceInfoListExW - try 3

[Detlef Riekenberg]

Hi Christian Gmeiner:
> Sorry for the new version, but i got rid of the whitspace stuff and also
> added a check if Reserved != NULL.
>
> I hope that this patch is ready to commit now.

+    if (MachineName != NULL)
+    {
+        FIXME("remote support is not implemented");
+        goto cleanup;
+    }

A failure should always set an error.
You should add a test for this in the testsuite.
(winspool as example uses an empty servername the same way as NULL).

For external Servers, ERROR_ACCESS_DENIED is a very common Result.

+    if (Reserved != NULL)
+    {
+        SetLastError(ERROR_INVALID_PARAMETER);
+        goto cleanup;
+    }
Same here. This is simple to test in the testsuite.


-        SerialPortList *list = (SerialPortList *)devinfo;
+        struct DeviceInfoSet *list = (struct DeviceInfoSet *)devinfo;
 
-        if (list->magic == SETUP_SERIAL_PORT_MAGIC)
+        if (list->magic == SETUP_DEVICE_INFO_SET_MAGIC)
         {

This is wrong.
Read: static HDEVINFO SETUP_CreateSerialDeviceList(void) (line 904)

---------------
In your test:

+    // create empty DeviceInfoList

C++ comments are not allow (not supported by some compiler)

+    devlist = SetupDiCreateDeviceInfoListExW(NULL, NULL, NULL, NULL);
+
+    ok(devlist && devlist != INVALID_HANDLE_VALUE,
"SetupDiCreateDeviceInfoListExW failed : %p %ld\n", devlist,
GetLastError());

- You missed the "SetLastError(0xdeadbeef);" before the tested function.
- The first Part of your "ok()" - message
  ("SetupDiCreateDeviceInfoListExW failed :") has no additional infos.
  When a test fails, you will already get:
  "__FILE__:__LINE__: Test failed:"
  You know the dll, file and line of the failing "ok()" - statement.
  Example: 
info.c:932: Test failed: returned 0 with 1801 (expected '!=0' or '0'
with ERROR_INVALID_PARAMETER)


You may want to look at my recent Tests / Implementations for
GetPrintProcessorDirectory or AddMonitor/DeleteMonitor (winspool.drv),
but do not use the macro "MAGIC_DEAD" in the tests
(it works, but 0xdeadbeef has more advantages).


As an Example, that we really need tests for every API-Function:
The second Parameter of "DeleteMonitor" is ignored in all tested 
Windows-Versions.  



-- 
By By ...
      ... Detlef

[Setuapi] implement SetupDiCreateDeviceInfoListExW - try 3

[Christian Gmeiner]

On Wednesday 14 June 2006 01:45, Detlef Riekenberg wrote:

Hi Detlef
> Hi Christian Gmeiner:
> > Sorry for the new version, but i got rid of the whitspace stuff and also
> > added a check if Reserved != NULL.
> >
> > I hope that this patch is ready to commit now.
>
> +    if (MachineName != NULL)
> +    {
> +        FIXME("remote support is not implemented");
> +        goto cleanup;
> +    }
>
> A failure should always set an error.
> You should add a test for this in the testsuite.
> (winspool as example uses an empty servername the same way as NULL).
>
> For external Servers, ERROR_ACCESS_DENIED is a very common Result.

For this case I will run a some tests on windows to see what it retuns.
> +    if (Reserved != NULL)
> +    {
> +        SetLastError(ERROR_INVALID_PARAMETER);
> +        goto cleanup;
> +    }
> Same here. This is simple to test in the testsuite.
>
> -        SerialPortList *list = (SerialPortList *)devinfo;
> +        struct DeviceInfoSet *list = (struct DeviceInfoSet *)devinfo;
>
> -        if (list->magic == SETUP_SERIAL_PORT_MAGIC)
> +        if (list->magic == SETUP_DEVICE_INFO_SET_MAGIC)
>          {
>
> This is wrong.
> Read: static HDEVINFO SETUP_CreateSerialDeviceList(void) (line 904)

Sure... I want to make a generic setupapi.dll and I will remove old serial 
device list, but as too big patches wont get accepted, I will do this in one
of my next patches.
If the basic setupapi stuff is done, I will implement hal handling into
explorer so that the regestry is filled in a correct way. And this regestry
values are used in setupapi.dll. This is why I want to a generic setupapi.dll.
> ---------------
> In your test:
>
> +    // create empty DeviceInfoList
>
> C++ comments are not allow (not supported by some compiler)

ok.. will fix it...
> +    devlist = SetupDiCreateDeviceInfoListExW(NULL, NULL, NULL, NULL);
> +
> +    ok(devlist && devlist != INVALID_HANDLE_VALUE,
> "SetupDiCreateDeviceInfoListExW failed : %p %ld\n", devlist,
> GetLastError());
>
> - You missed the "SetLastError(0xdeadbeef);" before the tested function.
> - The first Part of your "ok()" - message
>   ("SetupDiCreateDeviceInfoListExW failed :") has no additional infos.
>   When a test fails, you will already get:
>   "__FILE__:__LINE__: Test failed:"
>   You know the dll, file and line of the failing "ok()" - statement.
>   Example:
> info.c:932: Test failed: returned 0 with 1801 (expected '!=0' or '0'
> with ERROR_INVALID_PARAMETER)
>
> You may want to look at my recent Tests / Implementations for
> GetPrintProcessorDirectory or AddMonitor/DeleteMonitor (winspool.drv),
> but do not use the macro "MAGIC_DEAD" in the tests
> (it works, but 0xdeadbeef has more advantages).
>
> As an Example, that we really need tests for every API-Function:
> The second Parameter of "DeleteMonitor" is ignored in all tested
> Windows-Versions.

Ah.. i will extend my patch, run tests on windows and hope it will get it into 
cvs.

Greets,
Christian

-- 
Christian Gmeiner - student of computer science

http://dxr3plugin.sf.net
Wanna listen to good music? - http://www.purevolume.com/sinceyouaredead