Fix fprintf calls on Windows - #27
Conversation
| } else { // SymFromAddr() failed, just print the address | ||
| fprintf(fp, "%2d: [pc=0x%p]\n", i, pSymbol->Address); | ||
| fprintf(fp, "%2d: [pc=0x%p]\n", i, | ||
| reinterpret_cast<void*>(pSymbol->Address)); |
There was a problem hiding this comment.
If SymFromAddr() fails, is pSymbol->Address valid here?
@rnchamberlain
There was a problem hiding this comment.
I suspect that line was copied from 4 lines above, where SymFromAddr() has succeeded. The SymFromAddr() call might copy the address into pSymbol->Address in failure cases, but yes, it would be better/more logical to use dwAddress instead at this point. Can you add that to your fix? Thanks.
46dfc27 to
356c58a
Compare
| dwOffset, line.FileName, line.LineNumber); | ||
| } else { | ||
| // SymGetLineFromAddr64() failed, just print the address and symbol | ||
| if (dwOffset64 <= 32) { // sanity check |
There was a problem hiding this comment.
@rnchamberlain Is this sanity check valid? It seems to be consistently failing for me and we drop into the else clause.
There was a problem hiding this comment.
nb. Consistently dropping into the else clause here is why only test-api.js is failing in #26 as it's succeeding the SymGetLineFromAddr64 call -- The other tests fail the call and then fail the sanity check. The original fprintf for the else clause of the sanity check doesn't have parameters after the pSymbol->Address argument, so there the fprintf call there didn't crash.
pSymbol->Address is ULONG64 but fprintf's %p format on x86 expects 32-bits. Fixes nodejs#26
356c58a to
84cb4e4
Compare
|
Updated. I've dropped the sanity check as it seemed wrong and prevents the name of the method being printed. |
|
@richardlau I can't remember why that |
|
Landed as d988b6c |
4 participants
pSymbol->AddressisULONG64butfprintf's%pformat on x86 expects 32-bits.Also wrapped the long edited lines.
Fixes #26