jirentabu / crashrpt

Automatically exported from code.google.com/p/crashrpt
0 stars 0 forks source link

crAddScreenshot2() nJpegQuality is only used for the first error report then it is always set to 0 #217

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hi,

I have good info and repro steps below.  But at the high level 
CCrashHandler::PackCrashInfoIntoSharedMem() does not pack m_nJpegQuality.  So 
m_pTmpCrashDesc->m_nJpegQuality is set to 0 by memset(m_pTmpCrashDesc, 0, 
sizeof(CRASH_DESCRIPTION)).  Shortly after the first call to 
CCrashHandler::PackCrashInfoIntoSharedMem() users will call crAddScreenshot2() 
whose implementation directly updates m_pCrashDesc->m_nJpegQuality to correct 
quality.  Thus the system is initialised for the first error report.  However 
when using manual reports (crGenerateErrorReport()) to produce a second error 
report after the first report CCrashHandler::PackCrashInfoIntoSharedMem() is 
called from PerCrashInit() to reset everything which memsets 
m_pTmpCrashDesc->m_nJpegQuality to 0.  No call to crAddScreenshot2 is going to 
happen to set it correctly.

BTW - Very nice crash reporting software.  I have encountered some other more 
serious issues that make crGenerateErrorReport() fail after first use.  I think 
I've seen issues about that, when I get the energy I will add to them or create 
a new issue as necessary.

Cheers,
David

What steps will reproduce the problem?

1. We can reproduce the issue using WTLDemo from CrashRpt_vs2010.sln in v1.4.2

2. But first we need to be able to attach a debugger to CrashSender1402.exe 
when WTLDemo launches it before it inits itself.  I achieve this by placing a 
windows MessageBox in CrashSender to 'pause' it at startup so I can attach my 
debugger.
    File: CrashSender.cpp
    Function:     Run()
    Line (39):    Add this line ::MessageBoxA(0, "Some Text", "Some Caption", MB_OK);

3. Lets place some breakpoint in CrashSender's CrashInfoReader:
     File:        CrashInfoReader.cpp
     Function:    CCrashInfoReader::UnpackCrashDescription
     Line (560):  m_nJpegQuality = m_pCrashDesc->m_nJpegQuality

4. Run WTLDemo

5. Change Exception type to Manual report (so we can submit multiple error 
reports) and press Crash! button.

6. CrashSender.exe will show a window message box displaying "Some Text".  
Attach a vs2010 debugger to it, then press OK on the message box.

7. The debugger should stop at our breakpoint in UnpackCrashDescription().

8. Examine the value of m_pCrashDesc->m_nJpegQuality.  It will be 10 which is 
what crAddScreenshot2() was called with.  THIS IS GOOD. :)  But it only works 
on the first error report.

9. Continue to run CrashSender and close it when you can.

10. Go back to WTLDemo and press Crash! button again to generate a second error 
report.

11. Again attach the debugger to CrashSender.exe when the MessageBox appears 
and press OK button.

12. This time when we stop at the breakpoint, m_pCrashDesc->m_nJpegQuality 
value will be 0.  THIS IS NOT SO GOOD. :)  From this point forwards all error 
reports will be using nJpegQuality 0.

What is the expected output?

- that the correct nJpegQuality is used for every error report

What do you see instead?

- that the correct nJpegQuality is only used for first error report, after 
which nJpegQuality 0 is always sent by CrashHandler code

What version of CrashRpt are you using?

- 1.4.2

What is your version of Visual Studio?

- VS2010, Service Pack 1

What is your version of Windows operating system?

- Windows 7 64-bit

Please provide any additional information below.

Original issue reported on code.google.com by mrdavidf...@gmail.com on 2 Oct 2013 at 3:27

GoogleCodeExporter commented 9 years ago
It seems I could fix this by adding the following to CrashHandler.cpp:549:

m_pTmpCrashDesc->m_nJpegQuality = m_nJpegQuality;

Fixed in trunk

Original comment by zexspect...@gmail.com on 5 Oct 2013 at 4:40

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r1634.

Original comment by zexspect...@gmail.com on 5 Oct 2013 at 4:42