Bug in toaster example?

In WinDDK/7600.16385.0/src/general/toaster/kmdf/bus/dynamic/busenum.c
(@721) we have the following construct: (paraphrased…)

if (NT_STATUS(status)) {
status = WdfRegistryQueryULong(…)
if (NT_SUCCESS(status)) {
value = …
else
return STATUS_SUCCESS;

WdfRegistryClose(…)
}

Soooo… if WdfRegistryQueryULong() fails, we leave the registry open,
right? bug?

The reason for the confusion is that at line 634 we have:

status = WdfChildListRetrieveNextDevice(…)
if (!NT_STATUS(status) || status == STATUS_NO_MORE_ENTRIES) {
break
}

I understand that status is a packed variable, however the latter case
implies that we can “successfully find nothing” and the former case
implies we are guaranteed to find the object we are searching for in the
registry.

See my confusion?

Thx,
-PWM

Peter W. Morreale wrote:

In WinDDK/7600.16385.0/src/general/toaster/kmdf/bus/dynamic/busenum.c
(@721) we have the following construct: (paraphrased…)

if (NT_STATUS(status)) {
status = WdfRegistryQueryULong(…)
if (NT_SUCCESS(status)) {
value = …
else
return STATUS_SUCCESS;

WdfRegistryClose(…)
}

Soooo… if WdfRegistryQueryULong() fails, we leave the registry open,
right? bug?

Yes, that’s a bug. hKey is local to that function, so no one can clean
it up. The WdfRegistryClose should be moved to before the “if” statement.

The reason for the confusion is that at line 634 we have:

status = WdfChildListRetrieveNextDevice(…)
if (!NT_STATUS(status) || status == STATUS_NO_MORE_ENTRIES) {
break
}

I understand that status is a packed variable, however the latter case
implies that we can “successfully find nothing” and the former case
implies we are guaranteed to find the object we are searching for in the
registry.

That snippet also has a bug, although it is benign.
STATUS_NO_MORE_ENTRIES is a failure status, so it will always by caught
by the !NT_SUCCESS(status) half. The second half serves no purpose.
It’s like saying if( i > 9 || i == 22 ).

However, that chunk isn’t really related to the first chunk, is it? The
second chunk is in Bus_EjectDevice. There isn’t any way that could be
called if there were no devices to begin with.


Tim Roberts, xxxxx@probo.com
Providenza & Boekelheide, Inc.

On Mon, 2009-10-26 at 10:08 -0700, Tim Roberts wrote:

That snippet also has a bug, although it is benign.
STATUS_NO_MORE_ENTRIES is a failure status, so it will always by caught
by the !NT_SUCCESS(status) half. The second half serves no purpose.
It’s like saying if( i > 9 || i == 22 ).

nod. Wasn’t completely sure since the status is packed. Seemed odd in
combination with the registry query as was noted.

However, that chunk isn’t really related to the first chunk, is it? The
second chunk is in Bus_EjectDevice. There isn’t any way that could be
called if there were no devices to begin with.

No, not directly.

It is related only in that both were wrt query operations and the status
was not parsed consistently. Thus the question formed…

Thx
-PWM