Prev: OID_802_11_BSSID_LIST_SCAN
Next: get unicode character OEMTextOut
From: Mark Roddy on 31 May 2005 07:41
Wan-Hi wrote:
> 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.
>

Why on earth would you think that? What happens when a user pulls a
flash drive out? Where's the volume then? Server systems are frequently
attached to or contain storage enclosures that allow for hot
insertion/removal of physical disk devices. Of course you have to test
for errors.

> 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


If an api can return an error you need to check for the error condition.
However you do have a valid point regarding 'readablility' of the
code. The error handling can certainly clutter up code with tests for
conditions that are unlikely to happen. The problem of course is that
'unlikely' does not mean 'never'. Oh well.

I tend to encapsulate common sequences of API calls in subroutines (or
in C++ classes) which bury the ugliness in a cleaner interface.

>
> "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).


--

=====================
Mark Roddy DDK MVP
Windows 2003/XP/2000 Consulting
Hollis Technology Solutions 603-321-1032
www.hollistech.com
From: Wan-Hi on 31 May 2005 07:48
i understand your point of viewas a professional, but you should consider the
possibility that there are some amateurs among the professional majority. i
for myself deal with the field of device io just for fun and know no senior
programmer i can ask.

coding in a clean style is not the problem. what bothers me are cases like
if blocks in if blocks in if blocks and so on, just because the api function
i call *might* return a failure code (i assume in some cases this could only
happen if i passed a nullpointer for example). but for the sake of robustness
i live with the thousand ifs in ifs because i can never be sure if a
different circumstance causes a failure.

i think it's good to hear different point of views from the pros to weight
the arguments and find an own way how to deal with things.

"David J. Craig" wrote:

> ... 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.
> ...
From: Mark Roddy on 31 May 2005 07:50
David J. Craig wrote:
> 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.
>
>
>
or:

do {

error = stuff();
if (error) {
...
break;
}

error = morestuff;
if (error) {
...
break;
}
} while(0);


or the try/finally/leave stuff, although it has overhead.
The for and while one pass models break down (pun intended) over the
semantics of 'break'.


I vote for single/entrance single/exit coding style every time.

--

=====================
Mark Roddy DDK MVP
Windows 2003/XP/2000 Consulting
Hollis Technology Solutions 603-321-1032
www.hollistech.com
From: David J. Craig on 31 May 2005 11:50
Have you run PC-Lint on that construct? It is not valid K&R semantics in
that the terminating condition is fixed. I don't necessarily agree with the
rules, but it is what they gave us. I know many who use it, but I prefer
clean lint output and will avoid constructs that upset it. I also use the
maximum warning level, too.

"Mark Roddy" <markr(a)hollistech.com> wrote in message
news:usiBxbdZFHA.2076(a)TK2MSFTNGP15.phx.gbl...
> David J. Craig wrote:
>> 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.
>>
>>
>>
> or:
>
> do {
>
> error = stuff();
> if (error) {
> ...
> break;
> }
>
> error = morestuff;
> if (error) {
> ...
> break;
> }
> } while(0);
>
>
> or the try/finally/leave stuff, although it has overhead.
> The for and while one pass models break down (pun intended) over the
> semantics of 'break'.
>
>
> I vote for single/entrance single/exit coding style every time.
>
> --
>
> =====================
> Mark Roddy DDK MVP
> Windows 2003/XP/2000 Consulting
> Hollis Technology Solutions 603-321-1032
> www.hollistech.com


From: David J. Craig on 31 May 2005 11:55
On another issue, thanks for the ddkbuild system. I had my own
implementation and have mostly switched to yours. SlickEdit tends to run
some stuff auto-magically that cause the bscmake option to not work. I had
to add some help to my batch file that undoes it. They seem to call setenv
themselves if you set up the project based upon the DDK/IFS Kit.

"Mark Roddy" <markr(a)hollistech.com> wrote in message
news:usiBxbdZFHA.2076(a)TK2MSFTNGP15.phx.gbl...
> David J. Craig wrote:
>> 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.
>>
>>
>>
> or:
>
> do {
>
> error = stuff();
> if (error) {
> ...
> break;
> }
>
> error = morestuff;
> if (error) {
> ...
> break;
> }
> } while(0);
>
>
> or the try/finally/leave stuff, although it has overhead.
> The for and while one pass models break down (pun intended) over the
> semantics of 'break'.
>
>
> I vote for single/entrance single/exit coding style every time.
>
> --
>
> =====================
> Mark Roddy DDK MVP
> Windows 2003/XP/2000 Consulting
> Hollis Technology Solutions 603-321-1032
> www.hollistech.com


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