ZwQuerySystemInformation crash

I have a function that determines processname from the process id inside
a tdi driver. The code seems to get the process name correctly in most
cases, but the problem is that after this function is called ‘n’ times,
my system blue screens with the 7e error. The blue screen appears to
happen at random times, and does not depend on running any particular
process. The entire code snippet is given below. Can someone give me
pointers as to what could be going wrong?

Some issues are:

  1. I had to hard code the definitions of system structures used by
    ZwQuerySystemInformation.
  2. There is a for(;:wink: which I’m not sure is a good idea inside tdi
    code.

I got the original function from
http://www.alexfedotov.com/samples/threads.asp

Regards,

-Charu

#define MIN(a,b) ((a) > (b) ? (b) : (a))

/* Function to determine current process name in the kernel */

int MyTdi_GetCurrentProcessName(char *ProcessName)

{

ULONG cbBuffer = 0x10000; // declare initial size of buffer

  • 64

NTSTATUS Status;

PSYSTEM_PROCESS_INFORMATION pInfo;

KIRQL irql;

int ret = FALSE;

__try {

ULONG pid = (ULONG)PsGetCurrentProcessId();

DbgPrint(“MyTdi_GetCurrentProcessName\r\n”);

KeAcquireSpinLock(&ns_getprocname_lock, &irql);

do

{

if(!gBuffer)

gBuffer = ExAllocatePool
(NonPagedPool, cbBuffer); //allocate memory for the buffer of size
cbBuffer

if (gBuffer == NULL) // if memory allocation
failed, exit

{

DbgPrint(“MyTdi_GetCurrentProcessName: no memory\r\n”);

goto done;

}

// try to obtain system information into the
buffer

Status = ZwQuerySystemInformation(

SystemProcessesAndThreadsInformation, gBuffer, cbBuffer, NULL);

// if the size of the information is larger than
the size of the buffer

if (Status == STATUS_INFO_LENGTH_MISMATCH)

{

ExFreePool(gBuffer); // free the
memory associated with the buffer

gBuffer = NULL;

cbBuffer *= 2; // and increase
buffer size twice its original size

DbgPrint(“MyTdi_GetCurrentProcessName: realloc 0x%x\r\n”, cbBuffer);

}

else if (!NT_SUCCESS(Status)) // if operation is
not succeeded by any other reason

{

DbgPrint(“MyTdi_GetCurrentProcessName: zwqsi err 0x%x\r\n”, Status);

goto done;

}

}

while (Status == STATUS_INFO_LENGTH_MISMATCH);

pInfo = (PSYSTEM_PROCESS_INFORMATION)gBuffer;

for (;:wink: // forever do

{

PWSTR pszProcessName;

int len = 0;

short size = 0;

if (!pInfo) {

DbgPrint(“No more pInfo\r\n”);

break;

}

if (pInfo->ProcessId == pid) // check its
process ID against the pid we are looking for

{ // if they matched

// Check if name exists

if (!pInfo->ProcessName.Length ||
!pInfo->ProcessName.Buffer) {

// if no name available

DbgPrint(“No name found,
using default\r\n”);

strcpy(ProcessName,
“idle”); // set it to something

break;

}

// convert wide character string
“pszProcessName” to character string “ProcessName”

pszProcessName =
pInfo->ProcessName.Buffer; // assign a process name to a new variable

size = pInfo->ProcessName.Length /
2;

DbgPrint(“Before wcstombs, Buf 0x%x,
length %d\r\n”, pszProcessName, size);

len =
wcstombs(ProcessName,pszProcessName,MIN(MAX_PATH,pInfo->ProcessName.Leng
th));

DbgPrint(“After wcstombs %d\r\n”,
len);

if((len > 0) && (len < MAX_PATH)) {

ProcessName[len] = 0;

ret = TRUE;

}

else {

DbgPrint(“wcstombs
failed %d\r\n”, len);

ProcessName[0] = 0;

}

break; // exit the loop

}

if (pInfo->NextEntryDelta == 0) // if there are
no other entries in pInfo

break; // exit the loop

// if we are still in the loop, current entry
does not contain

// the process we are looking for, but there is
at least one more entry in pInfo

pInfo =
(PSYSTEM_PROCESS_INFORMATION)(((PUCHAR)pInfo)+ pInfo->NextEntryDelta);
// obtain that new entry

}

done:

KeReleaseSpinLock(&ns_getprocname_lock, irql);

DbgPrint(“MyTdi_GetCurrentProcessName: ret %d\r\n”, ret);

return ret; // and exit

}

__except(1) {

DbgPrint(“MyTdi_GetCurrentProcessName
EXCEPTION!!!\r\n”);

return FALSE;

}

return FALSE;

}


From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Developer
Sent: Wednesday, December 07, 2005 12:16 PM
To: Windows System Software Devs Interest List
Subject: Re: [ntdev] Printer Warning Dialogbox

If I use the ICopyHook COM extension in the windows Shell to tap all
copy, move, rename etc operations in the Windows explorer, and set the
CopyCallBack function to notify me of these events, then will this
callback be triggered if I copy/move/rename any file from the command
prompt program cmd.exe. Or do I have to hook somewhere else for tapping
those?
— Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256 You are currently subscribed
to ntdev as: unknown lmsubst tag argument: ‘’ To unsubscribe send a
blank email to xxxxx@lists.osr.com

This is absolutely horrible code, taking a quick look:

-You acquire a spinlock and hold it on for ages, not clear is why
-You are calling ZwQuerySystemInformation at DISPATCH_LEVEL instead of at
PASSIVE_LEVEL.
-You use ZwQuerySystemInformation which is not documented
-Try and error on the buffer size and doubling it if it fails is a very poor
practice
-The use of goto is poor practice
-Instead of wcstomb you should use RtlUnicodeStringToAnsi and properly
calculate the size of the ansi string or get it allocated automatically. The
size of multi byte strings are not always equal to the number of characters.

I could surely go on like this for a while, but I think it is maybe better
that you spend some energy on your education instead.

Regards,

Daniel Terhell
Resplendence Software Projects Sp
xxxxx@resplendence.com
http://www.resplendence.com


“Charu Venkatraman” wrote in message
news:xxxxx@ntdev…
I have a function that determines processname from the process id inside a
tdi driver. The code seems to get the process name correctly in most cases,
but the problem is that after this function is called ‘n’ times, my system
blue screens with the 7e error. The blue screen appears to happen at random
times, and does not depend on running any particular process. The entire
code snippet is given below. Can someone give me pointers as to what could
be going wrong?
Some issues are:
I had to hard code the definitions of system structures used by
ZwQuerySystemInformation.
There is a for(;:wink: which I’m not sure is a good idea inside tdi code.
I got the original function from
http://www.alexfedotov.com/samples/threads.asp

Regards,
-Charu

xxxxx@lists.osr.com

this code is not needed if you get processname when you receive TDI_CONNECT - then it can be stored in connection context which will be associated with address object context; later, if you receive packets at tdi layer, you’ll know the process

Petr Kurtin

If you look up the function in MSDN, it tells you how to know how big of
a buffer to allocate. I also want to reiterate the calling IRQL.
Although the doc page does not mention IRQL, I know of very few (if any)
ZwXxxx calls that are callable at IRQL == DISPATCH_LEVEL.

d

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Daniel Terhell
Sent: Wednesday, December 07, 2005 4:17 AM
To: Windows System Software Devs Interest List
Subject: Re:[ntdev] ZwQuerySystemInformation crash

This is absolutely horrible code, taking a quick look:

-You acquire a spinlock and hold it on for ages, not clear is why
-You are calling ZwQuerySystemInformation at DISPATCH_LEVEL instead of
at
PASSIVE_LEVEL.
-You use ZwQuerySystemInformation which is not documented
-Try and error on the buffer size and doubling it if it fails is a very
poor
practice
-The use of goto is poor practice
-Instead of wcstomb you should use RtlUnicodeStringToAnsi and properly
calculate the size of the ansi string or get it allocated automatically.
The
size of multi byte strings are not always equal to the number of
characters.

I could surely go on like this for a while, but I think it is maybe
better
that you spend some energy on your education instead.

Regards,

Daniel Terhell
Resplendence Software Projects Sp
xxxxx@resplendence.com
http://www.resplendence.com


“Charu Venkatraman” wrote in message
news:xxxxx@ntdev…
I have a function that determines processname from the process id inside
a
tdi driver. The code seems to get the process name correctly in most
cases,
but the problem is that after this function is called ‘n’ times, my
system
blue screens with the 7e error. The blue screen appears to happen at
random
times, and does not depend on running any particular process. The entire

code snippet is given below. Can someone give me pointers as to what
could
be going wrong?
Some issues are:
I had to hard code the definitions of system structures used by
ZwQuerySystemInformation.
There is a for(;:wink: which I’m not sure is a good idea inside tdi code.
I got the original function from
http://www.alexfedotov.com/samples/threads.asp

Regards,
-Charu

xxxxx@lists.osr.com


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: xxxxx@microsoft.com
To unsubscribe send a blank email to xxxxx@lists.osr.com

Furthermore, your use of exception handling is a bit scary. You should
not be using it at all. You can be masking off system errors that
should be bugchecks much earlier then your 0x7e. The code you have can
orphan the spinlock for instance

d

-----Original Message-----
From: Doron Holan
Sent: Wednesday, December 07, 2005 6:26 AM
To: ‘Windows System Software Devs Interest List’
Subject: RE: [ntdev] ZwQuerySystemInformation crash

If you look up the function in MSDN, it tells you how to know how big of
a buffer to allocate. I also want to reiterate the calling IRQL.
Although the doc page does not mention IRQL, I know of very few (if any)
ZwXxxx calls that are callable at IRQL == DISPATCH_LEVEL.

d

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Daniel Terhell
Sent: Wednesday, December 07, 2005 4:17 AM
To: Windows System Software Devs Interest List
Subject: Re:[ntdev] ZwQuerySystemInformation crash

This is absolutely horrible code, taking a quick look:

-You acquire a spinlock and hold it on for ages, not clear is why
-You are calling ZwQuerySystemInformation at DISPATCH_LEVEL instead of
at
PASSIVE_LEVEL.
-You use ZwQuerySystemInformation which is not documented
-Try and error on the buffer size and doubling it if it fails is a very
poor
practice
-The use of goto is poor practice
-Instead of wcstomb you should use RtlUnicodeStringToAnsi and properly
calculate the size of the ansi string or get it allocated automatically.
The
size of multi byte strings are not always equal to the number of
characters.

I could surely go on like this for a while, but I think it is maybe
better
that you spend some energy on your education instead.

Regards,

Daniel Terhell
Resplendence Software Projects Sp
xxxxx@resplendence.com
http://www.resplendence.com


“Charu Venkatraman” wrote in message
news:xxxxx@ntdev…
I have a function that determines processname from the process id inside
a
tdi driver. The code seems to get the process name correctly in most
cases,
but the problem is that after this function is called ‘n’ times, my
system
blue screens with the 7e error. The blue screen appears to happen at
random
times, and does not depend on running any particular process. The entire

code snippet is given below. Can someone give me pointers as to what
could
be going wrong?
Some issues are:
I had to hard code the definitions of system structures used by
ZwQuerySystemInformation.
There is a for(;:wink: which I’m not sure is a good idea inside tdi code.
I got the original function from
http://www.alexfedotov.com/samples/threads.asp

Regards,
-Charu

xxxxx@lists.osr.com


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: xxxxx@microsoft.com
To unsubscribe send a blank email to xxxxx@lists.osr.com

The underlying NtQuerySystemInformation is pageable code, plus he is
doing widechar to char conversions, which runs through pageable
translation tables. Its all bad.

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Doron Holan
Sent: Wednesday, December 07, 2005 9:26 AM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash

If you look up the function in MSDN, it tells you how to know how big of
a buffer to allocate. I also want to reiterate the calling IRQL.
Although the doc page does not mention IRQL, I know of very few (if any)
ZwXxxx calls that are callable at IRQL == DISPATCH_LEVEL.

d

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Daniel Terhell
Sent: Wednesday, December 07, 2005 4:17 AM
To: Windows System Software Devs Interest List
Subject: Re:[ntdev] ZwQuerySystemInformation crash

This is absolutely horrible code, taking a quick look:

-You acquire a spinlock and hold it on for ages, not clear is why
-You are calling ZwQuerySystemInformation at DISPATCH_LEVEL instead of
at
PASSIVE_LEVEL.
-You use ZwQuerySystemInformation which is not documented
-Try and error on the buffer size and doubling it if it fails is a very
poor
practice
-The use of goto is poor practice
-Instead of wcstomb you should use RtlUnicodeStringToAnsi and properly
calculate the size of the ansi string or get it allocated automatically.
The
size of multi byte strings are not always equal to the number of
characters.

I could surely go on like this for a while, but I think it is maybe
better
that you spend some energy on your education instead.

Regards,

Daniel Terhell
Resplendence Software Projects Sp
xxxxx@resplendence.com
http://www.resplendence.com


“Charu Venkatraman” wrote in message
news:xxxxx@ntdev…
I have a function that determines processname from the process id inside
a
tdi driver. The code seems to get the process name correctly in most
cases,
but the problem is that after this function is called ‘n’ times, my
system
blue screens with the 7e error. The blue screen appears to happen at
random
times, and does not depend on running any particular process. The entire

code snippet is given below. Can someone give me pointers as to what
could
be going wrong?
Some issues are:
I had to hard code the definitions of system structures used by
ZwQuerySystemInformation.
There is a for(;:wink: which I’m not sure is a good idea inside tdi code.
I got the original function from
http://www.alexfedotov.com/samples/threads.asp

Regards,
-Charu

xxxxx@lists.osr.com


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: xxxxx@microsoft.com
To unsubscribe send a blank email to xxxxx@lists.osr.com


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag argument:
‘’
To unsubscribe send a blank email to xxxxx@lists.osr.com

Charu Venkatraman wrote:

I have a function that determines processname from the process id
inside a tdi driver. The code seems to get the process name correctly
in most cases, but the problem is that after this function is called
?n? times, my system blue screens with the 7e error. The blue screen
appears to happen at random times, and does not depend on running any
particular process. The entire code snippet is given below. Can
someone give me pointers as to what could be going wrong?

#define MIN(a,b) ((a) > (b) ? (b) : (a))

/* Function to determine current process name in the kernel */

int MyTdi_GetCurrentProcessName(char *ProcessName)

{

ULONG cbBuffer = 0x10000; // declare initial size of buffer - 64

NTSTATUS Status;

PSYSTEM_PROCESS_INFORMATION pInfo;

KIRQL irql;

int ret = FALSE;

__try {

ULONG pid = (ULONG)PsGetCurrentProcessId();

DbgPrint(“MyTdi_GetCurrentProcessName\r\n”);

KeAcquireSpinLock(&ns_getprocname_lock, &irql);

THAT is your key problem. Once you acquire the spinlock, you run at an
elevated IRQL, and several of the interfaces you call (such as
ZwQuerySystemInformation and wcstombs) cannot be called at an elevated
IRQL. That will result in an eventual, although unpredictable, blue screen.

I don’t understand why you acquire the spin lock at all. It doesn’t seem
to be necessary. And why do you convert back to ANSI? Why can’t you just
keep the Unicode name around?


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

> ----------

From: xxxxx@lists.osr.com[SMTP:xxxxx@lists.osr.com] on behalf of Daniel Terhell[SMTP:xxxxx@resplendence.com]
Reply To: Windows System Software Devs Interest List
Sent: Wednesday, December 07, 2005 1:17 PM
To: Windows System Software Devs Interest List
Subject: Re:[ntdev] ZwQuerySystemInformation crash

-The use of goto is poor practice

Although I agree the code you commented is horrible, I don’t agree with above statement. Actually, goto is used quite correctly in this piece of code. Sometimes it is necessary to make cleanup before returning from function and goto is good for this purpose. In user mode I prefer __try/__finally and __leave but it isn’t a good idea in drivers. Other possibility is do {…} while (FALSE); and break; but it is somewhat limited if there is a switch statement or nested loop.

BTW, what you said is often claimed by teachers who never wrote any piece of code for real use :wink: It is right only for goto back in attempt to simulate loop. Goto forward may or may not be correct. Personally, I take direct return from the middle of function (as used in most DDK examples) as much worse practice than goto. It has to be changed to goto when one wants traces (to display function parameters on enter and return value on exit).

Best regards,

Michal Vodicka
UPEK, Inc.
[xxxxx@upek.com, http://www.upek.com]

So I agree and I simply hide the goto label as a macro named LEAVE and
the FINALLY as a macro that create the label that matches the goto
statement and TRY as a macro that resolves to nothing at all.

TRY
{
Somecode;
If (badthing)
{
LEAVE; // really the dreaded goto
}
Morecode;
}
FINALLY // the label for the goto
{
Cleanupthismess();
}

And then everyone is happy. While(0) has issues with code checkers that
don’t think it is 100% real C.

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
Sent: Wednesday, December 07, 2005 3:50 PM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash


From:
xxxxx@lists.osr.com[SMTP:xxxxx@lists.osr.com
] on behalf of Daniel Terhell[SMTP:xxxxx@resplendence.com]
Reply To: Windows System Software Devs Interest List
Sent: Wednesday, December 07, 2005 1:17 PM
To: Windows System Software Devs Interest List
Subject: Re:[ntdev] ZwQuerySystemInformation crash

-The use of goto is poor practice

Although I agree the code you commented is horrible, I don’t agree with
above statement. Actually, goto is used quite correctly in this piece of
code. Sometimes it is necessary to make cleanup before returning from
function and goto is good for this purpose. In user mode I prefer
__try/__finally and __leave but it isn’t a good idea in drivers. Other
possibility is do {…} while (FALSE); and break; but it is somewhat
limited if there is a switch statement or nested loop.

BTW, what you said is often claimed by teachers who never wrote any
piece of code for real use :wink: It is right only for goto back in attempt
to simulate loop. Goto forward may or may not be correct. Personally, I
take direct return from the middle of function (as used in most DDK
examples) as much worse practice than goto. It has to be changed to goto
when one wants traces (to display function parameters on enter and
return value on exit).

Best regards,

Michal Vodicka
UPEK, Inc.
[xxxxx@upek.com, http://www.upek.com]


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag argument:
‘’
To unsubscribe send a blank email to xxxxx@lists.osr.com

And now someone will point out preprocessor macros are evil :wink: Yes, this is the way to go; it can be even more readable than pure goto Done I routinely use.

As for while(0), I guess I have a rule in PC-lint options which turns this warning off. Prefast doesn’t allow it?

Best regards,

Michal Vodicka
UPEK, Inc.
[xxxxx@upek.com, http://www.upek.com]


From: xxxxx@lists.osr.com[SMTP:xxxxx@lists.osr.com] on behalf of Roddy, Mark[SMTP:xxxxx@stratus.com]
Reply To: Windows System Software Devs Interest List
Sent: Wednesday, December 07, 2005 10:16 PM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash

So I agree and I simply hide the goto label as a macro named LEAVE and
the FINALLY as a macro that create the label that matches the goto
statement and TRY as a macro that resolves to nothing at all.

TRY
{
Somecode;
If (badthing)
{
LEAVE; // really the dreaded goto
}
Morecode;
}
FINALLY // the label for the goto
{
Cleanupthismess();
}

And then everyone is happy. While(0) has issues with code checkers that
don’t think it is 100% real C.

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
Sent: Wednesday, December 07, 2005 3:50 PM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash

> ----------
> From:
xxxxx@lists.osr.com[SMTP:xxxxx@lists.osr.com
] on behalf of Daniel Terhell[SMTP:xxxxx@resplendence.com]
> Reply To: Windows System Software Devs Interest List
> Sent: Wednesday, December 07, 2005 1:17 PM
> To: Windows System Software Devs Interest List
> Subject: Re:[ntdev] ZwQuerySystemInformation crash
>
> -The use of goto is poor practice
>
Although I agree the code you commented is horrible, I don’t agree with
above statement. Actually, goto is used quite correctly in this piece of
code. Sometimes it is necessary to make cleanup before returning from
function and goto is good for this purpose. In user mode I prefer
__try/__finally and __leave but it isn’t a good idea in drivers. Other
possibility is do {…} while (FALSE); and break; but it is somewhat
limited if there is a switch statement or nested loop.

BTW, what you said is often claimed by teachers who never wrote any
piece of code for real use :wink: It is right only for goto back in attempt
to simulate loop. Goto forward may or may not be correct. Personally, I
take direct return from the middle of function (as used in most DDK
examples) as much worse practice than goto. It has to be changed to goto
when one wants traces (to display function parameters on enter and
return value on exit).

Best regards,

Michal Vodicka
UPEK, Inc.
[xxxxx@upek.com, http://www.upek.com]


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag argument:
‘’
To unsubscribe send a blank email to xxxxx@lists.osr.com


Questions? First check the Kernel Driver FAQ at http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag argument: ‘’
To unsubscribe send a blank email to xxxxx@lists.osr.com

Looks like a GOTO, walks like a GOTO, smells like a GOTO, but i’snot a
GOTO huh?

It sounds to me like a Rose by any other name is still a rose.

Gary G. Little

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Roddy, Mark
Sent: Wednesday, December 07, 2005 3:17 PM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash

So I agree and I simply hide the goto label as a macro named LEAVE and
the FINALLY as a macro that create the label that matches the goto
statement and TRY as a macro that resolves to nothing at all.

TRY
{
Somecode;
If (badthing)
{
LEAVE; // really the dreaded goto
}
Morecode;
}
FINALLY // the label for the goto
{
Cleanupthismess();
}

And then everyone is happy. While(0) has issues with code checkers that
don’t think it is 100% real C.

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
Sent: Wednesday, December 07, 2005 3:50 PM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash


From:
xxxxx@lists.osr.com[SMTP:xxxxx@lists.osr.com
] on behalf of Daniel Terhell[SMTP:xxxxx@resplendence.com]
Reply To: Windows System Software Devs Interest List
Sent: Wednesday, December 07, 2005 1:17 PM
To: Windows System Software Devs Interest List
Subject: Re:[ntdev] ZwQuerySystemInformation crash

-The use of goto is poor practice

Although I agree the code you commented is horrible, I don’t agree with
above statement. Actually, goto is used quite correctly in this piece of
code. Sometimes it is necessary to make cleanup before returning from
function and goto is good for this purpose. In user mode I prefer
__try/__finally and __leave but it isn’t a good idea in drivers. Other
possibility is do {…} while (FALSE); and break; but it is somewhat
limited if there is a switch statement or nested loop.

BTW, what you said is often claimed by teachers who never wrote any
piece of code for real use :wink: It is right only for goto back in attempt
to simulate loop. Goto forward may or may not be correct. Personally, I
take direct return from the middle of function (as used in most DDK
examples) as much worse practice than goto. It has to be changed to goto
when one wants traces (to display function parameters on enter and
return value on exit).

Best regards,

Michal Vodicka
UPEK, Inc.
[xxxxx@upek.com, http://www.upek.com]


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag argument:
‘’
To unsubscribe send a blank email to xxxxx@lists.osr.com


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: unknown lmsubst tag argument: ‘’
To unsubscribe send a blank email to xxxxx@lists.osr.com

Any compiler (or linter) that chokes on “do { … } while(0)” is simply
broken. It’s 100% valid C. It’s also one of the more reliable ways to make
macros that correctly handle “;” statement termination.

– arlie

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
Sent: Wednesday, December 07, 2005 4:36 PM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash

And now someone will point out preprocessor macros are evil :wink: Yes, this is
the way to go; it can be even more readable than pure goto Done I routinely
use.

As for while(0), I guess I have a rule in PC-lint options which turns this
warning off. Prefast doesn’t allow it?

Best regards,

Michal Vodicka
UPEK, Inc.
[xxxxx@upek.com, http://www.upek.com]

PC-lint doesn’t choke, it just informs about it. From docs (717 is informational message, not warning):

717 do … while(0) – Whereas this represents a constant in a context expecting a Boolean, this
construct is probably a deliberate attempt on the part of the programmer to encapsulate a
sequence of statements into a single statement, and so it is given a separate error message. [22
section 20.7] For example:
#define f(k) do {n=k; m=n+1;} while(0)
allows f(k) to be used in conditional statements as in
if(n>0) f(3);
else f(2);
Thus, if you are doing this deliberately use -e717

Best regards,

Michal Vodicka
UPEK, Inc.
[xxxxx@upek.com, http://www.upek.com]


From: xxxxx@lists.osr.com[SMTP:xxxxx@lists.osr.com] on behalf of Arlie Davis[SMTP:xxxxx@stonestreetone.com]
Reply To: Windows System Software Devs Interest List
Sent: Wednesday, December 07, 2005 10:44 PM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash

Any compiler (or linter) that chokes on “do { … } while(0)” is simply
broken. It’s 100% valid C. It’s also one of the more reliable ways to make
macros that correctly handle “;” statement termination.

– arlie

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
Sent: Wednesday, December 07, 2005 4:36 PM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash

And now someone will point out preprocessor macros are evil :wink: Yes, this is
the way to go; it can be even more readable than pure goto Done I routinely
use.

As for while(0), I guess I have a rule in PC-lint options which turns this
warning off. Prefast doesn’t allow it?

Best regards,

Michal Vodicka
UPEK, Inc.
[xxxxx@upek.com, http://www.upek.com]


Questions? First check the Kernel Driver FAQ at http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: xxxxx@upek.com
To unsubscribe send a blank email to xxxxx@lists.osr.com

According to K&R, page 57, it says in part: "If the test, expr2, is not
present, it is taken as permanently true, so for (;;){ …} is an “infinite”
loop, presumably to be broken by other means (such as a break or return).
PC-Lint chokes on while(0) because it is a constant. I prefer the for(;:wink:
{ … break;} style, especially for a block where most tests require a break
out, with cleanup to be done. Using SEH with just the __try{}, __leave; and
__finally{} blocks is another solution in Microsoft centric drivers.

The use of gotos is bad because it leaves no trace of how flow happened. If
you work at writing without using ‘goto’ you will find that your design
skills will improve and you will be able to think much more globally. I
found this true over 30 years ago when using COBOL where ‘go to’ could go
anywhere within the code. It was a big help in working with assembler and
my later switch to C and even later C++. It makes you realize that having
no way to trace code flow is a really bad idea.

“Arlie Davis” wrote in message
news:xxxxx@ntdev…
> Any compiler (or linter) that chokes on “do { … } while(0)” is simply
> broken. It’s 100% valid C. It’s also one of the more reliable ways to
> make
> macros that correctly handle “;” statement termination.
>
> – arlie
>
>
> -----Original Message-----
> From: xxxxx@lists.osr.com
> [mailto:xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
> Sent: Wednesday, December 07, 2005 4:36 PM
> To: Windows System Software Devs Interest List
> Subject: RE: [ntdev] ZwQuerySystemInformation crash
>
> And now someone will point out preprocessor macros are evil :wink: Yes, this
> is
> the way to go; it can be even more readable than pure goto Done I
> routinely
> use.
>
> As for while(0), I guess I have a rule in PC-lint options which turns this
> warning off. Prefast doesn’t allow it?
>
> Best regards,
>
> Michal Vodicka
> UPEK, Inc.
> [xxxxx@upek.com, http://www.upek.com]
>
>
>
>

Does it warn if you use while(false)?

Phil

Philip D. Barila
Seagate Technology LLC
(720) 684-1842

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
Sent: Wednesday, December 07, 2005 3:07 PM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash

PC-lint doesn’t choke, it just informs about it. From docs (717 is
informational message, not warning):

717 do … while(0) – Whereas this represents a constant in a context
expecting a Boolean, this
construct is probably a deliberate attempt on the part of the programmer
to encapsulate a
sequence of statements into a single statement, and so it is given a
separate error message. [22
section 20.7] For example:
#define f(k) do {n=k; m=n+1;} while(0)
allows f(k) to be used in conditional statements as in
if(n>0) f(3);
else f(2);
Thus, if you are doing this deliberately use -e717

Best regards,

Michal Vodicka
UPEK, Inc.
[xxxxx@upek.com, http://www.upek.com]

Yes, it does. It warns because of constant boolean expression used as a condition. However, it is just informational message and can be turned off several ways. Globally disable all informational messages, globally turn off this message as docs suggests (-e717) or disable it locally when used (//lint !e717 after while(0)).

PC-lint is great tool when used and configured correctly (which may not be so easy). Even PeterGV changed his original opinion about it :wink:

Best regards,

Michal Vodicka
UPEK, Inc.
[xxxxx@upek.com, http://www.upek.com]


From: xxxxx@lists.osr.com[SMTP:xxxxx@lists.osr.com] on behalf of xxxxx@seagate.com[SMTP:xxxxx@seagate.com]
Reply To: Windows System Software Devs Interest List
Sent: Wednesday, December 07, 2005 11:18 PM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash

Does it warn if you use while(false)?

Phil

Philip D. Barila
Seagate Technology LLC
(720) 684-1842

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
Sent: Wednesday, December 07, 2005 3:07 PM
To: Windows System Software Devs Interest List
Subject: RE: [ntdev] ZwQuerySystemInformation crash

PC-lint doesn’t choke, it just informs about it. From docs (717 is
informational message, not warning):

717 do … while(0) – Whereas this represents a constant in a context
expecting a Boolean, this
construct is probably a deliberate attempt on the part of the programmer
to encapsulate a
sequence of statements into a single statement, and so it is given a
separate error message. [22
section 20.7] For example:
#define f(k) do {n=k; m=n+1;} while(0)
allows f(k) to be used in conditional statements as in
if(n>0) f(3);
else f(2);
Thus, if you are doing this deliberately use -e717

Best regards,

Michal Vodicka
UPEK, Inc.
[xxxxx@upek.com, http://www.upek.com]


Questions? First check the Kernel Driver FAQ at http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: xxxxx@upek.com
To unsubscribe send a blank email to xxxxx@lists.osr.com

Well then the use of try/finally is equally broken as one may be
substituted for the other.

SEH has massive stack overhead, even in the limited case of
try/leave/finally with no exception handler. It was the stack overhead,
staring me in the face in the form of a double fault blue screen, that
lead me to replace SEH usage in the kernel with gotos masquerading as
SEH. Try using it in the lowest driver in a large stack of drivers and
get back to me about your experience.

Functions written with try/finally/leave semantics can generally
completely avoid deeply nested conditionals (typical no nested if
statements at all), and provide single entry/single exit semantics,
which allow code reviewers to readily check resource
allocation/deallocation correctness. Just because some academic
pontificated 30 years ago ‘thou shalt not’ doesn’t really mean much in
the real world. Gotos are bad if they are used badly.

p.s. the next time that Outlook decides to replace SEH with SHE I am
going to shoot it :slight_smile:

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of David J. Craig
Sent: Wednesday, December 07, 2005 5:20 PM
To: Windows System Software Devs Interest List
Subject: Re:[ntdev] ZwQuerySystemInformation crash

According to K&R, page 57, it says in part: "If the test, expr2, is not

present, it is taken as permanently true, so for (;;){ …} is an
“infinite”
loop, presumably to be broken by other means (such as a break or
return).
PC-Lint chokes on while(0) because it is a constant. I prefer the
for(;:wink:
{ … break;} style, especially for a block where most tests require a
break
out, with cleanup to be done. Using SEH with just the __try{}, __leave;
and
__finally{} blocks is another solution in Microsoft centric drivers.

The use of gotos is bad because it leaves no trace of how flow happened.
If
you work at writing without using ‘goto’ you will find that your design
skills will improve and you will be able to think much more globally. I

found this true over 30 years ago when using COBOL where ‘go to’ could
go
anywhere within the code. It was a big help in working with assembler
and
my later switch to C and even later C++. It makes you realize that
having
no way to trace code flow is a really bad idea.

“Arlie Davis” wrote in message
news:xxxxx@ntdev…
> Any compiler (or linter) that chokes on “do { … } while(0)” is
simply
> broken. It’s 100% valid C. It’s also one of the more reliable ways
to
> make
> macros that correctly handle “;” statement termination.
>
> – arlie
>
>
> -----Original Message-----
> From: xxxxx@lists.osr.com
> [mailto:xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
> Sent: Wednesday, December 07, 2005 4:36 PM
> To: Windows System Software Devs Interest List
> Subject: RE: [ntdev] ZwQuerySystemInformation crash
>
> And now someone will point out preprocessor macros are evil :wink: Yes,
this
> is
> the way to go; it can be even more readable than pure goto Done I
> routinely
> use.
>
> As for while(0), I guess I have a rule in PC-lint options which turns
this
> warning off. Prefast doesn’t allow it?
>
> Best regards,
>
> Michal Vodicka
> UPEK, Inc.
> [xxxxx@upek.com, http://www.upek.com]
>
>
>
>


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: xxxxx@stratus.com
To unsubscribe send a blank email to xxxxx@lists.osr.com

If C/C++ allowed, as COBOL 68 did, you to specify any label in any part of
the program as the object of a goto statement, would you consider its use to
be a good idea? Especially into other modules or into the middle of some
function in some other module in some library. I will presume not since I
know you are an experienced developer. I did in later years begin to use
‘go to’ with the only object permitted being the exit paragraph of a section
that was being ‘performed’. This was not completely required, but it did
eliminate priming reads which ‘go to’-less programming in Cobol requires.
Then, Cobol did not support any other form of structured loops, so that was
pretty much all there was available. I understand that recent versions of
Cobol have added more structure, but any language that requires that all
data be global has problems that I left behind over 18 years ago.

P.S. I don’t want to go back to Cobol.

P.P.S. You must be using Word as your editor in Outlook. I still use OE
since it does both email and newsgroups where I can save newsgroup messages
in my email folders when I deem them worthy or in some other way remarkable
for their usefullness or stupidity.

“Roddy, Mark” wrote in message news:xxxxx@ntdev…
Well then the use of try/finally is equally broken as one may be
substituted for the other.

SEH has massive stack overhead, even in the limited case of
try/leave/finally with no exception handler. It was the stack overhead,
staring me in the face in the form of a double fault blue screen, that
lead me to replace SEH usage in the kernel with gotos masquerading as
SEH. Try using it in the lowest driver in a large stack of drivers and
get back to me about your experience.

Functions written with try/finally/leave semantics can generally
completely avoid deeply nested conditionals (typical no nested if
statements at all), and provide single entry/single exit semantics,
which allow code reviewers to readily check resource
allocation/deallocation correctness. Just because some academic
pontificated 30 years ago ‘thou shalt not’ doesn’t really mean much in
the real world. Gotos are bad if they are used badly.

p.s. the next time that Outlook decides to replace SEH with SHE I am
going to shoot it :slight_smile:

-----Original Message-----
From: xxxxx@lists.osr.com
[mailto:xxxxx@lists.osr.com] On Behalf Of David J. Craig
Sent: Wednesday, December 07, 2005 5:20 PM
To: Windows System Software Devs Interest List
Subject: Re:[ntdev] ZwQuerySystemInformation crash

According to K&R, page 57, it says in part: "If the test, expr2, is not

present, it is taken as permanently true, so for (;;){ …} is an
“infinite”
loop, presumably to be broken by other means (such as a break or
return).
PC-Lint chokes on while(0) because it is a constant. I prefer the
for(;:wink:
{ … break;} style, especially for a block where most tests require a
break
out, with cleanup to be done. Using SEH with just the try{}, leave;
and
__finally{} blocks is another solution in Microsoft centric drivers.

The use of gotos is bad because it leaves no trace of how flow happened.
If
you work at writing without using ‘goto’ you will find that your design
skills will improve and you will be able to think much more globally. I

found this true over 30 years ago when using COBOL where ‘go to’ could
go
anywhere within the code. It was a big help in working with assembler
and
my later switch to C and even later C++. It makes you realize that
having
no way to trace code flow is a really bad idea.

“Arlie Davis” wrote in message
news:xxxxx@ntdev…
> Any compiler (or linter) that chokes on “do { … } while(0)” is
simply
> broken. It’s 100% valid C. It’s also one of the more reliable ways
to
> make
> macros that correctly handle “;” statement termination.
>
> – arlie
>
>
> -----Original Message-----
> From: xxxxx@lists.osr.com
> [mailto:xxxxx@lists.osr.com] On Behalf Of Michal Vodicka
> Sent: Wednesday, December 07, 2005 4:36 PM
> To: Windows System Software Devs Interest List
> Subject: RE: [ntdev] ZwQuerySystemInformation crash
>
> And now someone will point out preprocessor macros are evil :wink: Yes,
this
> is
> the way to go; it can be even more readable than pure goto Done I
> routinely
> use.
>
> As for while(0), I guess I have a rule in PC-lint options which turns
this
> warning off. Prefast doesn’t allow it?
>
> Best regards,
>
> Michal Vodicka
> UPEK, Inc.
> [xxxxx@upek.com, http://www.upek.com]
>
>
>
>


Questions? First check the Kernel Driver FAQ at
http://www.osronline.com/article.cfm?id=256

You are currently subscribed to ntdev as: xxxxx@stratus.com
To unsubscribe send a blank email to xxxxx@lists.osr.com

David J. Craig wrote:

I understand that recent versions of
Cobol have added more structure, but any language that requires that all
data be global has problems that I left behind over 18 years ago.

You’ve probably heard the joke:
Hey, did you hear about the new object-oriented version of COBOL?
It’s called “ADD ONE TO COBOL”.


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

Mark Roddy wrote:

SEH has massive stack overhead, even in the limited case of
try/leave/finally with no exception handler. It was the stack
overhead,

I’m curious. How much is “massive”? i.e. if I add a __try … __finally round
the outside of a function, by how many bytes does it’s stack usage go up?
Sure, I could measure it myself, but you must have already done that.

On a separate point, the replacement of __try … __finally with macro
calls that look the same but have different semantics makes me nervous.

(What do I mean by different semantics? Consider what happens if a
maintainer inserts a return into the TRY … FINALLY block).

If you always maintain your stuff, it probably isn’t a problem but we
can all recall cases where later maintainers just thought they knew
what macros did without looking at the definitions because “it was obvious
from the name”. I would fear that TRY and FINALLY are an accident waiting to
happen (unless, as I said, you do your own maintenance).

Don