http://breakpad.appspot.com/48001/diff/1/3 File client/linux/handler/exception_handler.cc (right): http://breakpad.appspot.com/48001/diff/1/3#newcode86 Line 86: #include "client/linux/minidump_writer//minidump_writer.h" request for a drive by fix ...
http://breakpad.appspot.com/48001/diff/1/3
File client/linux/handler/exception_handler.cc (right):
http://breakpad.appspot.com/48001/diff/1/3#newcode86
Line 86: #include "client/linux/minidump_writer//minidump_writer.h"
request for a drive by fix to remove the duplicate forward slash :-)
http://breakpad.appspot.com/48001/diff/1/3#newcode382
Line 382:
ideally, the existing code that handles signals could call into the same method
that on demand generation does.
but, now that i've suggested combine code paths, i also think it might be
worthwhile to have the page allocator be statically defined and to reuse the
same memory for the cloned thread's stack across multiple invocations of
WriteMinidump.
http://breakpad.appspot.com/48001/diff/1/2
File processor/testdata/linux_test_app.cc (right):
http://breakpad.appspot.com/48001/diff/1/2#newcode80
Line 80: }
Does this test case not fit well into the model of using Google Test? It's part
of the tree in src/testing/...
http://breakpad.appspot.com/48001/diff/1/3
File client/linux/handler/exception_handler.cc (right):
http://breakpad.appspot.com/48001/diff/1/3#newcode86
Line 86: #include "client/linux/minidump_writer//minidump_writer.h"
On 2009/12/14 21:09:24, mochalatte wrote:
> request for a drive by fix to remove the duplicate forward slash :-)
Done.
http://breakpad.appspot.com/48001/diff/1/3#newcode382
Line 382:
On 2009/12/14 21:09:24, mochalatte wrote:
> ideally, the existing code that handles signals could call into the same
method
> that on demand generation does.
>
> but, now that i've suggested combine code paths, i also think it might be
> worthwhile to have the page allocator be statically defined and to reuse the
> same memory for the cloned thread's stack across multiple invocations of
> WriteMinidump.
I've refactored the code to combine the code paths. What are the motivations to
move to a statically defined page allocator?
http://breakpad.appspot.com/48001/diff/1/2
File processor/testdata/linux_test_app.cc (right):
http://breakpad.appspot.com/48001/diff/1/2#newcode39
Line 39: // generate an executable with STABS symbols (needs -m32), or -gdwarf-2
for one // with DWARF symbols (32- or 64-bit)
I split this line.
http://breakpad.appspot.com/48001/diff/1/2#newcode80
Line 80: }
On 2009/12/14 21:09:24, mochalatte wrote:
> Does this test case not fit well into the model of using Google Test? It's
part
> of the tree in src/testing/...
This app is a test application that exercises code (like a test) but can also be
used to generate symbols, etc., like the Windows-specific test_app.cc in this
directory.
few more nits :-) http://breakpad.appspot.com/48001/diff/1/3 File client/linux/handler/exception_handler.cc (right): http://breakpad.appspot.com/48001/diff/1/3#newcode382 Line 382: On 2009/12/14 23:38:45, brdevmn ...
few more nits :-)
http://breakpad.appspot.com/48001/diff/1/3
File client/linux/handler/exception_handler.cc (right):
http://breakpad.appspot.com/48001/diff/1/3#newcode382
Line 382:
On 2009/12/14 23:38:45, brdevmn wrote:
> On 2009/12/14 21:09:24, mochalatte wrote:
> > ideally, the existing code that handles signals could call into the same
> method
> > that on demand generation does.
> >
> > but, now that i've suggested combine code paths, i also think it might be
> > worthwhile to have the page allocator be statically defined and to reuse the
> > same memory for the cloned thread's stack across multiple invocations of
> > WriteMinidump.
>
> I've refactored the code to combine the code paths. What are the motivations
to
> move to a statically defined page allocator?
>
I guess I thought it'd be cleaner to have the minidump-generating thread at the
same stack address, but I suppose that opens it up to being corrupted by the
child process.
http://breakpad.appspot.com/48001/diff/2003/3003#newcode354
Line 354: ThreadArgument thread_arg;
Since the minidump writing is synchronous and the callback happens at the end,
is it necessary for WiteMinidump to support a callback functionality?
Additionally, any particular reason to have a static method rather than an
instance member of an existing ExceptionHandler object that the client app can
use?
http://breakpad.appspot.com/48001/diff/2003/3003 File client/linux/handler/exception_handler.cc (right): http://breakpad.appspot.com/48001/diff/2003/3003#newcode354 Line 354: On 2009/12/15 06:53:29, mochalatte wrote: > Since the ...
http://breakpad.appspot.com/48001/diff/2003/3003
File client/linux/handler/exception_handler.cc (right):
http://breakpad.appspot.com/48001/diff/2003/3003#newcode354
Line 354:
On 2009/12/15 06:53:29, mochalatte wrote:
> Since the minidump writing is synchronous and the callback happens at the end,
> is it necessary for WiteMinidump to support a callback functionality?
It's not strictly necessary, but it may allow for slightly easier mixed-mode use
by being consistent between on-demand and on-exception minidump generation.
>
> Additionally, any particular reason to have a static method rather than an
> instance member of an existing ExceptionHandler object that the client app can
> use?
Both forms are provided to be consistent with the Windows and Mac codebases.
http://breakpad.appspot.com/48001/diff/2003/3003
File client/linux/handler/exception_handler.cc (right):
http://breakpad.appspot.com/48001/diff/2003/3003#newcode354
Line 354:
On 2009/12/15 21:38:26, brdevmn wrote:
> On 2009/12/15 06:53:29, mochalatte wrote:
> > Since the minidump writing is synchronous and the callback happens at the
end,
> > is it necessary for WiteMinidump to support a callback functionality?
>
> It's not strictly necessary, but it may allow for slightly easier mixed-mode
use
> by being consistent between on-demand and on-exception minidump generation.
> >
I'm going to have to be difficult here :-) One scenario I can imagine is using
the callback to track the number of crashes; if we do callbacks in both the
ondemand case and the on-exception case, the client of Breakpad will have to
keep track of when they've initiated an ondemand dump to make sure they aren't
tracking this statistic incorrectly.
Maybe we could add an 'on-demand' boolean to the callback?
LGTM
http://breakpad.appspot.com/48001/diff/2003/3003
File client/linux/handler/exception_handler.cc (right):
http://breakpad.appspot.com/48001/diff/2003/3003#newcode354
Line 354:
On 2009/12/16 19:43:36, mochalatte wrote:
> On 2009/12/15 21:38:26, brdevmn wrote:
> > On 2009/12/15 06:53:29, mochalatte wrote:
> > > Since the minidump writing is synchronous and the callback happens at the
> end,
> > > is it necessary for WiteMinidump to support a callback functionality?
> >
> > It's not strictly necessary, but it may allow for slightly easier mixed-mode
> use
> > by being consistent between on-demand and on-exception minidump generation.
> > >
>
> I'm going to have to be difficult here :-) One scenario I can imagine is using
> the callback to track the number of crashes; if we do callbacks in both the
> ondemand case and the on-exception case, the client of Breakpad will have to
> keep track of when they've initiated an ondemand dump to make sure they aren't
> tracking this statistic incorrectly.
>
> Maybe we could add an 'on-demand' boolean to the callback?
>
>
I guess since they specify a context for each on demand generation, the client
can use that parameter for this purpose. Please disregard, I'll LGTM
Issue 48001: Added on-demand dump generation for Linux, Linux test app
Created 14 years, 11 months ago by brdevmn
Modified 14 years, 11 months ago
Reviewers: mochalatte
Base URL: http://google-breakpad.googlecode.com/svn/trunk/src/
Comments: 12