Prev: OID_802_11_BSSID_LIST_SCAN
Next: get unicode character OEMTextOut
From: Wan-Hi on 29 May 2005 18:06
originated in the java corner, i have some difficulties to adapt the
programming style used in the platform sdk and the ddk. therefore i'd like to
know what you think about the way i handled some things.

1) device interface GUIDs not defined in a global header:
i tried to find the GUID of disc type devices which are part of the mass
storage device group. because i didn't find any, i looked up the GUID value
stored in my registry and defined it in my app. is this a bad habit or can i
assume that this GUID never changes in further versions of ms win?

2) error checking:
SetupDiGetClassDevs and many other functions return a specific value if they
fail. so should i check for failure although such case is impossible? e.g.
SetupDiGetClassDevs with GUID_DEVINTERFACE_VOLUME should never fail. i ask
because useless failure checks make the code unreadable.
From: Don Burn on 30 May 2005 10:46
Comments inline:

"Wan-Hi" <WanHi(a)discussions.microsoft.com> wrote in message
news:2647E6F7-D684-47A9-B75D-10E7A9407F55(a)microsoft.com...
> originated in the java corner, i have some difficulties to adapt the
> programming style used in the platform sdk and the ddk. therefore i'd like
> to
> know what you think about the way i handled some things.
>
> 1) device interface GUIDs not defined in a global header:
> i tried to find the GUID of disc type devices which are part of the mass
> storage device group. because i didn't find any, i looked up the GUID
> value
> stored in my registry and defined it in my app. is this a bad habit or can
> i
> assume that this GUID never changes in further versions of ms win?

Device Interface GUID's are defined in header files, search for DEFINE_GUID
in the DDK and you will find them all defined.


> 2) error checking:
> SetupDiGetClassDevs and many other functions return a specific value if
> they
> fail. so should i check for failure although such case is impossible? e.g.
> SetupDiGetClassDevs with GUID_DEVINTERFACE_VOLUME should never fail. i ask
> because useless failure checks make the code unreadable.

Never assume that an error can't occur, this will just lead to a bug in the
future. While the current documentation is pretty good for the SDK and DDK,
I still have encountered errors that are not defined in docs. Normally,
these are events that are rare, but at least handling the error (if nothing
else check for it and exit if failure displaying the error code.

What do you mean that failure checks make the code unreadable, show us an
example of what you consider unreadable, and I suspect the group will have a
number of suggestions in coding style that will make things clearer (since
everyones style is different, choose something that works for you).


--
Don Burn (MVP, Windows DDK)
Windows 2k/XP/2k3 Filesystem and Driver Consulting
Remove StopSpam from the email to reply



From: Wan-Hi on 30 May 2005 21:15
here are two examples. the first one checks for possible failures:

HDEVINFO hVolInfoSet;
HDEVINFO hDiscInfoSet

// Retrieving the device info set of currently installed
// devices exposing the volume interface.
hVolInfoSet = SetupDiGetClassDevs(&GUID_DEVINTERFACE_VOLUME,
NULL,
NULL,
DIGCF_DEVICEINTERFACE | DIGCF_PRESENT);
if (hVolInfoSet == INVALID_HANDLE_VALUE)
{
ShowErrorMessage(GetLastError());
return false;
}


// Retrieving the device info set of currently installed
devices registered as part of the disc type setup class.
hDiscInfoSet = SetupDiGetClassDevs(&GUID_DEVCLASS_DISKDRIVE,
NULL,
NULL,
DIGCF_PRESENT);
if (hDiscInfoSet == INVALID_HANDLE_VALUE)
{
ShowErrorMessage(GetLastError());
SetupDiDestroyDeviceInfoList(hVolInfoSet);
return false;
}

....

// Iterating through all volumes.
for (DWORD i = 0; ; i++)
{

// Retrieving the interface data of the next entry in the
// device info set of all volumes.
if (SetupDiEnumDeviceInterfaces(hVolInfoSet,
NULL,
&GUID_DEVINTERFACE_VOLUME,
i,
&volInterfaceData) == 0)
{
errorCode = GetLastError();
if (errorCode == ERROR_NO_MORE_ITEMS)
break;
else
{
ShowErrorMessage(errorCode);
continue;
}
}
...
}
....

I think this is very over-secure code because volumes and disc type devices
should always be present. therefore no failure should happen.

the second example leaves out failure checks because a condition is
fulfilled which should make it impossible to fail further API calls:

....
// Examing the device node in the device tree.
ULONG devNodeDepth;
if (CM_Get_Depth(&devNodeDepth, volDevInfoData.DevInst, 0) != CR_SUCCESS)
{
free(pVolDetailData);
ShowErrorMessage(GetLastError());
continue;
}

if (devNodeDepth >= 2)
{
///////////////////////////////////////////////////////
// 1) RETRIEVING DEVICE INSTANCE IDs
DEVINST hDiscClassInst;
DEVINST hUsbClassInst;
ULONG volIdSize;
ULONG discIdSize;
ULONG usbIdSize;
TCHAR *volId;
TCHAR *discId;
TCHAR *usbId;

// Getting the handles of the device interface's parent and
// grandparent.
CM_Get_Parent(&hDiscClassInst, volDevInfoData.DevInst, NULL);
CM_Get_Parent(&hUsbClassInst, hDiscClassInst, NULL);

// Determing memory requirements for the device IDs and
// allocating the required space.
CM_Get_Device_ID_Size(&volIdSize, volDevInfoData.DevInst, 0);
CM_Get_Device_ID_Size(&discIdSize, hDiscClassInst, 0);
CM_Get_Device_ID_Size(&usbIdSize, hUsbClassInst, 0);
volId = (TCHAR*)malloc((volIdSize + 1) * sizeof(TCHAR));
discId = (TCHAR*)malloc((discIdSize + 1) * sizeof(TCHAR));
usbId = (TCHAR*)malloc((usbIdSize + 1) * sizeof(TCHAR));

// Getting the device IDs.
CM_Get_Device_ID(volDevInfoData.DevInst, volId, volIdSize, 0);
CM_Get_Device_ID(hDiscClassInst, discId, discIdSize, 0);
CM_Get_Device_ID(hUsbClassInst, usbId, usbIdSize, 0);
...
(freeing allocated mem)
}
....

I skipped failure checks on CM_Get_Parent because the depth of the device
node tells that the device instance definitely has an sufficient 'amount' of
elders. assuming that CM_Get_Parent wont fail i also skip failure checks on
CM_Get_Device_ID_Size and CM_Get_Device_ID because every device instance must
have a unique ID associated.

by these two examples i tried to explain what kind of programming style i
exercise. i try to check for failures if i haven't have enough information to
assume a successful function call. otherwise, if i have enough information
because i checked specific conditions, i leave the checks out.

is this adequate or unsave?

wan-hi

"Don Burn" wrote:

> Never assume that an error can't occur, this will just lead to a bug in the
> future. While the current documentation is pretty good for the SDK and DDK,
> I still have encountered errors that are not defined in docs. Normally,
> these are events that are rare, but at least handling the error (if nothing
> else check for it and exit if failure displaying the error code.
>
> What do you mean that failure checks make the code unreadable, show us an
> example of what you consider unreadable, and I suspect the group will have a
> number of suggestions in coding style that will make things clearer (since
> everyones style is different, choose something that works for you).
From: Robert Marquardt on 31 May 2005 01:53
Wan-Hi wrote:

> 2) error checking:
> SetupDiGetClassDevs and many other functions return a specific value if they
> fail. so should i check for failure although such case is impossible? e.g.
> SetupDiGetClassDevs with GUID_DEVINTERFACE_VOLUME should never fail. i ask
> because useless failure checks make the code unreadable.

The error checks only make the code unreadable because MS prefers a
*really* bad style. Excessive return and goto is about as bad as it can
get. A clean if else style and a proper indentation solves that.
From: David J. Craig on 31 May 2005 02:16
Here is a freebie for those trying to do error checking and avoiding goto's.

K&R say the only legal 'DO ONCE' is done using the for command.

for( ; ; )
{
break; // If OE or ??? warps this, it should be four lines
with this line indented.
}

You can then do tests and do a 'continue' or a 'break' as needed to exit or
restart the loop. You will forget the terminating break especially when
beginning, but it is legal and works. You can add a __finally to handle
terminating cleanup, but I just usually initialize all locals and return
variables before the 'for' block. Then a test and the appropriate cleanup
can be done to free memory, close handles, etc. after exiting the block or
before returning.

If you can't code it clean and easy to follow, you need to go back to school
or find something other than a programming job. No one will live forever,
much less stay in the same job, so be nice to those who follow and write
maintainable code. You can add comments to the 'for' loop to tell anyone
looking that this is a 'do once' block and not to get confused.

"Robert Marquardt" <marquardt(a)codemercs.com> wrote in message
news:%23TSRLUaZFHA.3732(a)TK2MSFTNGP10.phx.gbl...
> Wan-Hi wrote:
>
>> 2) error checking:
>> SetupDiGetClassDevs and many other functions return a specific value if
>> they fail. so should i check for failure although such case is
>> impossible? e.g. SetupDiGetClassDevs with GUID_DEVINTERFACE_VOLUME should
>> never fail. i ask because useless failure checks make the code
>> unreadable.
>
> The error checks only make the code unreadable because MS prefers a
> *really* bad style. Excessive return and goto is about as bad as it can
> get. A clean if else style and a proper indentation solves that.


 |  Next  |  Last
Pages: 1 2 3 4 5
Prev: OID_802_11_BSSID_LIST_SCAN
Next: get unicode character OEMTextOut