Fixing some more code analysis issues.

[sgstair]

• Updated IRQL modification annotations
• Expanded some 32bit additions to 64bit in statistics
• Corrected some edge cases tools were complaining about.

[cron2]

This generally speaking looks good. So ACK.

I do wonder about the ASSERT(ethernetHeader); before the "if (ethernetHeader == NULL)" check - what does the ASSERT() macro do here? In OpenVPN code (which has other headers) the ASSERT() would already trigger on ethernetHeader being NULL, aborting the program.

[sgstair]

The ASSERT macro is compiled out in the release build (shipping driver) - but in a debug build it will break into the debugger to inform developers about a failed condition. I left it in so devs can be alerted to problems in future development. In Windows drivers, there are no abort mechanisms besides bugchecking the system and taking everything down, so all asserts continue by design. They also generally continue without notice unless you have a debugger or something watching debug messages connected.

[cron2]

Hi,

On Mon, Jul 01, 2019 at 12:54:11PM -0700, Stephen Stair wrote:

The ASSERT macro is compiled out in the release build (shipping driver) - but in a debug build it will break into the debugger to inform developers about a failed condition. I left it in so devs can be alerted to problems in future development. In Windows drivers, there are no abort mechanisms besides bugchecking the system and taking everything down, so all asserts continue by design. They also generally continue without notice unless you have a debugger or something watching debug messages connected.

Thanks for the explanation. This makes sense :-)

gert

-- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany gert@greenie.muc.de