Issue 3664002: [MIPS] Fix core dump related unit tests for Android on MIPS

Issue 3664002: [MIPS] Fix core dump related unit tests for Android on MIPS (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
9 years, 11 months ago by gordanac

Modified:
9 years, 11 months ago

Reviewers:
Ivan Penkov, digit, Mark Mentovai, Ben Chan

CC:
Petar Jovanovic, petar.jovanovic_imgtec.com

Base URL:
http://google-breakpad.googlecode.com/svn/trunk/

Visibility:
Public.

More Reviews

Description

This change fixes failing unittests in Android on MIPS:
LinuxCoreDumperTest.VerifyDumpWithMultipleThreads
ElfCoreDumpTest.ValidCoreFile

BUG=None
TEST=Running breakpad_unittests on MIPS Android device

Commited: https://code.google.com/p/google-breakpad/source/detail?r=1330

Patch Set 2 : Removing extra space #

Patch Set 3 : Set proper registers range #

Total comments: 3

Patch Set 4 : Addressing comments #

Total comments: 3

Patch Set 5 : Addressing latest comments #

Created: 9 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats			Patch
M	src/client/linux/minidump_writer/linux_core_dumper.cc	View	1 2 3 4	1 chunk	+12 lines, -0 lines	0 comments	Download
M	src/common/android/include/sys/procfs.h	View	1 2 3 4	2 chunks	+5 lines, -0 lines	0 comments	Download
M	src/common/android/include/sys/user.h	View	1 2 3	1 chunk	+48 lines, -0 lines	0 comments	Download

Messages

Total messages: 3

Expand All Messages | Collapse All Messages

Ben Chan

https://breakpad.appspot.com/3664002/diff/50002/src/client/linux/minidump_writer/linux_core_dumper.cc File src/client/linux/minidump_writer/linux_core_dumper.cc (right): https://breakpad.appspot.com/3664002/diff/50002/src/client/linux/minidump_writer/linux_core_dumper.cc#newcode192 src/client/linux/minidump_writer/linux_core_dumper.cc:192: int i = 0; this seems unnecessary https://breakpad.appspot.com/3664002/diff/50002/src/common/android/include/sys/procfs.h File ...

9 years, 11 months ago #1

Mark Mentovai

9 years, 11 months ago #3

LGTM otherwise

https://breakpad.appspot.com/3664002/diff/10004/src/client/linux/minidump_wri...
File src/client/linux/minidump_writer/linux_core_dumper.cc (right):

https://breakpad.appspot.com/3664002/diff/10004/src/client/linux/minidump_wri...
src/client/linux/minidump_writer/linux_core_dumper.cc:193: info.regs.regs[0] =
0;
I know that $0 always reads 0, but why not pull the 0 value from status->pr_reg
in the loop below?

https://breakpad.appspot.com/3664002/diff/10004/src/common/android/include/sy...
File src/common/android/include/sys/procfs.h (right):

https://breakpad.appspot.com/3664002/diff/10004/src/common/android/include/sy...
src/common/android/include/sys/procfs.h:101: unsigned long pr_uid;
Looks like everything else in here is aligned, can you put an extra space before
pr_uid and pr_gid?

Expand All Messages | Collapse All Messages