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.

Comments inline:

"Wan-Hi" <WanHi@discussions.microsoft.com> wrote in message
news:2647E6F7-D684-47A9-B75D-10E7A9407F55@microsoft.com...
Device Interface GUID's are defined in header files, search for DEFINE_GUID
in the DDK and you will find them all defined.


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

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:

Wan-Hi wrote:

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.

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@codemercs.com> wrote in message
news:%23TSRLUaZFHA.3732@TK2MSFTNGP10.phx.gbl...

Wan-Hi wrote:
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.


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.


--

=====================
Mark Roddy DDK MVP
Windows 2003/XP/2000 Consulting
Hollis Technology Solutions 603-321-1032
www.hollistech.com

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:

David J. Craig wrote:

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

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@hollistech.com> wrote in message
news:usiBxbdZFHA.2076@TK2MSFTNGP15.phx.gbl...

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@hollistech.com> wrote in message
news:usiBxbdZFHA.2076@TK2MSFTNGP15.phx.gbl...

If you are coding if blocks within if blocks more than three or so levels
deep, you need to reconsider your algorithms and design. It is not
necessary. It leads to bad results since the code becomes far more
difficult to debug, understand, and maintain. Most bad code can be fixed
with better algorithms and a redesign. That 'do once' block helps limit the
depth to which you must nest 'if' statements. Version 1.0 of most programs
and drivers can and should be discarded for a better design later.
Commercial pressures make most of us keep the old code, so then you just
have to improve it block at a time as you have the time or you need to
modify or fix a particular area.

"Wan-Hi" <WanHi@discussions.microsoft.com> wrote in message
news:837701A9-C56D-43B6-90AD-8414314E84D8@microsoft.com...

"Wan-Hi" <WanHi@discussions.microsoft.com> wrote in message news2CA4989-7791-46FC-8935-DA124B309607@microsoft.com...
Ok, if your app is not very mission-critical, and the error is unlikely,
how do you like following "style":

CHECK( some_call );
CHECK( another call );
CHECK( yet_another call );
.......

where CHECK is defined as:

inline bool Check_helper( BOOL retcode ) {
reurn retcode ? true: false;
)

inline bool Check_helper(HDEVINFO h) {
return h != INVALID_HANDLE_VALUE;
}


#define CHECK( expr ) if( !Check_helper(expr) ) return false

or throw exception instead of return.

--PA

"Robert Marquardt" <marquardt@codemercs.com> wrote in message news:#TSRLUaZFHA.3732@TK2MSFTNGP10.phx.gbl...
?? So where MS prefers a bad style? C++ is a standard language, you're free
to format your code as nice as it gets....

--PA

David J. Craig wrote:
I turn off warning C4127. For some reason I dislike for(;, always
have. I'm not really that fond of do()while(0) either. TRY/LEAVE/FINALLY
implemented as macros that avoid SEH and c++ exception handling work,
and solve the 'inner break' problem, but they don't nest, only one per
function, and they use the dreaded goto.



--

=====================
Mark Roddy DDK MVP
Windows 2003/XP/2000 Consulting
Hollis Technology Solutions 603-321-1032
www.hollistech.com

The latest PC-Lint flags goto as depreciated. I remember trying to avoid
them even in Cobol. I have been linting some old large code bases and
finding lots of interesting stuff. Most are just style type issues and of
course there are so many functions that return values, but you can't check
because you have no answer to the question of what to do if it fails. Very
few people like to use (void) to show an intentional omission.

This is just so much fun especially when you add drvfast, code coverage,
bounds checker, and much more. Hard shakedowns are necessary. I remember
my times at Iomega where Bernoulli drives were hooked up to a Mac. They
were strapped on to a plastic tray that lifted up to about 45 degrees and
then fell. The drives weren't done until that could run for several weeks
without a single unrecoverable error.

"Mark Roddy" <markr@hollistech.com> wrote in message
news:uCpFjBlZFHA.1040@TK2MSFTNGP10.phx.gbl...

Pavel A. wrote:

USBVIEW for example.

if (!success)
{
OOPS();
return NULL;
}

and four other checks with only different if expression in sequence.
How about a simple helper function like oops() returning NULL?

if (!success)
return oops();

Then a longer expression for the if reduces the number of lines without
losing clarity.

Also goto is used often. goto is always a sign of bad programming style.

The DDK examples are the blueprint for many commercial projects. They
should teach a better style.

On Wed, 01 Jun 2005 08:05:04 +0200, Robert Marquardt wrote:

I've seen this - and seen it debated - a lot. One could argue that the
four successive if blocks make the code easier to follow. Everything you
need to understand is in front of you. 'return oops()' forces the reader
to some other place in the code, or even another file to figure out the
functionality.

I don't have a concrete number in mind, but I would much rather sift
through a small number of if blocks. 37 blocks would certainly be too many
....

i admit you are definitely right. i did not go through many cases in mind.
just one of many things that makes me just an amateur and you a pro. i will
keep in mind to expect exceptional use cases. thanks.

"Mark Roddy" wrote:

the fact that excessive if block 'nesting' causes maintainance problems was
the very reason why i reconsidered my way of failure checks and asked you.

i don't really believe that it's generally caused by a bad algorithm.
logically

if (failure)
{
do clean up;
show error;
return;
}
else
{
if (nextFailure)
{
do clean up;
show error;
return;
}
else
{
...
}
}

is logically more correct than a code avoiding 'else' blocks. in my opinion,
functioning code avoiding 'else' blocks is a result and benefit of the
language specific 'code execution path'. so the algo is not that bad, but the
design is. this does not mean that i hold up the opinion to use 'else' if
unnecessary. i also second you that i should reconsider my design when i
encounter excessive if block 'nesting'.

"David Craig" wrote:

BobF wrote:

You are wrong. A function oops() or a macro OOPS() are equivalent in
understanding the functionality.
What i dislike is the 4 lines needed (plus the empty line). With 5
blocks you get a staggering 25 lines where about 5 would be sufficient.