google / UIforETW

User interface for recording and managing ETW traces
https://randomascii.wordpress.com/2015/04/14/uiforetw-windows-performance-made-easier/
Apache License 2.0
1.59k stars 201 forks source link

ControlOK in Utility.cpp should check return value of GetParent #42

Closed ariccio closed 9 years ago

ariccio commented 9 years ago

The line:

if (!IsWindowVisible(win) && IsWindowVisible(GetParent(win)))

Might trigger undefined behavior, as GetParent can fail, and then NULL is passed to IsWindowVisible.

It's most likely harmless, but nonetheless an easy fix.

randomascii commented 9 years ago

Probably best handled with an assert. If GetParent(win) returns NULL then either: a) There's a bug in UIforETW, in which case an assert is appropriate. b) Windows is in a near-death state, in which case recovery is probably not possible.

Trying to handle impossible errors leads to uncertainty about what errors are possible. In this case the contract for this function is that you are supposed to pass in the handle of a control, which has a parent, and an assert will document that contract.

ariccio commented 9 years ago

Fair enough.

Windows is in a near-death state

On the other hand, I imagine that some users of UIforETW, using it for diagnosing obscure/insidious performance issues, might actually be running in a near-death state. No idea what to do about it, but it's an interesting thought.

randomascii commented 9 years ago

Resolved by f89e5f9