Code review - Issue 3764002: Add a unit test for generating a minidump from a core dump on Linux.https://breakpad.appspot.com/2015-02-02T20:37:14+00:00rietveld
Message from unknown
2015-02-02T16:21:03+00:00Ben Chanurn:md5:6898c2649fe5fd846f3e844b9c34d1ca
Message from benchan@chromium.org
2015-02-02T16:21:42+00:00Ben Chanurn:md5:8863f6486182cf5e6ad93dcc76a5a07c
Message from unknown
2015-02-02T18:07:31+00:00Ben Chanurn:md5:c793fac1939f4de35dad21d0765dbc69
Message from vapier@chromium.org
2015-02-02T18:23:12+00:00vapierurn:md5:fdf82612f2cf480632032bb00ffcd9d3
https://breakpad.appspot.com/3764002/diff/30001/src/client/linux/minidump_writer/minidump_writer_unittest.cc
File src/client/linux/minidump_writer/minidump_writer_unittest.cc (right):
https://breakpad.appspot.com/3764002/diff/30001/src/client/linux/minidump_writer/minidump_writer_unittest.cc#newcode768
src/client/linux/minidump_writer/minidump_writer_unittest.cc:768: return;
shouldn't there be an ASSERT here instead of silently returning ?
Message from benchan@chromium.org
2015-02-02T19:53:06+00:00Ben Chanurn:md5:55648f8f5d577c754c999e85f3b5b124
https://breakpad.appspot.com/3764002/diff/30001/src/client/linux/minidump_writer/minidump_writer_unittest.cc
File src/client/linux/minidump_writer/minidump_writer_unittest.cc (right):
https://breakpad.appspot.com/3764002/diff/30001/src/client/linux/minidump_writer/minidump_writer_unittest.cc#newcode768
src/client/linux/minidump_writer/minidump_writer_unittest.cc:768: return;
On 2015/02/02 18:23:12, vapier wrote:
> shouldn't there be an ASSERT here instead of silently returning ?
it's not breakpad's fault if the core file isn't generated by the platform (e.g. the core pattern isn't correct, etc). other breakpad unit tests also does a simply return
Message from vapier@chromium.org
2015-02-02T20:37:14+00:00vapierurn:md5:d3c6890d7f336f635858803c500a626a
lgtm
https://breakpad.appspot.com/3764002/diff/30001/src/client/linux/minidump_writer/minidump_writer_unittest.cc
File src/client/linux/minidump_writer/minidump_writer_unittest.cc (right):
https://breakpad.appspot.com/3764002/diff/30001/src/client/linux/minidump_writer/minidump_writer_unittest.cc#newcode768
src/client/linux/minidump_writer/minidump_writer_unittest.cc:768: return;
it might be breakpad's fault :)
i tried grepping for existing examples before posting but failed to find any. many my grep wasn't doing what i thought it was.