Re: RFR: JDK-8235846: Improve WindbgDebuggerLocal implementation

Mon, 13 Jan 2020 16:40:30 -0800

Hi Alex,

It looks pretty good.
Just some minor comments below.

The class AutoCOMPtr has unfixed indents.
I guess, the function AutoArrayPtr.asPtr() is not used anymore and can be deleted.
 

I'd suggest to remove first level '()' brackets in the comment to avoid brackets nesting:
  74 // manage COM 'auto' pointers (to avoid multiple Release
  75 // calls at every early (exception) returns).
   =>
  74 // manage COM 'auto' pointers to avoid multiple Release
  75 // calls at every early (exception) returns.

I'd suggest to reformat it as it was originally formatted:
 297   if (IsEqualIID(interfaceId, __uuidof(IUnknown))
 298         || IsEqualIID(interfaceId, __uuidof(IDebugOutputCallbacks))) {
  =>
 297   if (IsEqualIID(interfaceId, __uuidof(IUnknown)) ||
 298       IsEqualIID(interfaceId, __uuidof(IDebugOutputCallbacks))) {

The comment about S_FALSE is not that clear as S_FALSE is not really used
(better to use consistently just one value: S_FALSE or false):
 418   // S_FALSE means timeout
 419   COM_VERIFY_OK_(ptrIDebugControl->WaitForEvent(DEBUG_WAIT_DEFAULT, INFINITE),
 420                  "Windbg Error: WaitForEvent failed!", false);

NULL is used in some places and nullptr in others.

Thanks,
Serguei


On 12/19/19 15:34, Alex Menkov wrote:
Hi all,

Please review a fix for
https://bugs.openjdk.java.net/browse/JDK-8235846
webrev:
http://cr.openjdk.java.net/~amenkov/jdk15/WinDbg_improve/webrev.01/

Main goal of the change is to improve error reporting (we have several bugs and need at least COM error codes for WinDbg calls).
Also the fix improves/rearranges this quite old code.

--alex

Reply via email to