Address additional comments from r1248. Fix the register to check and fix some style nits.

Address additional comments from r1248. Fix the register to check and fix some
style nits.

BUG=495
R=mark@chromium.org

Committed: https://code.google.com/p/google-breakpad/source/detail?r=1254

[Lei Zhang (chromium)]

I concur with your post-commit comments.

[Mark Mentovai]

LGTM.

https://breakpad.appspot.com/834003/diff/1/src/client/linux/minidump_writer/l...
File src/client/linux/minidump_writer/linux_ptrace_dumper.cc (right):

https://breakpad.appspot.com/834003/diff/1/src/client/linux/minidump_writer/l...
src/client/linux/minidump_writer/linux_ptrace_dumper.cc:199: if
(sys_ptrace(PTRACE_GETFPXREGS, tid, NULL, &info->fpxregs) == -1) {
Another option is to eschew the direct __cpuid stuff here. You could catch a
ptrace failure with errno set to ENODEV, accept that as legitimate as opposed to
failure, and memset the fpxregs field to 0.

[Lei Zhang (chromium)]

https://breakpad.appspot.com/834003/diff/1/src/client/linux/minidump_writer/l...
File src/client/linux/minidump_writer/linux_ptrace_dumper.cc (right):

https://breakpad.appspot.com/834003/diff/1/src/client/linux/minidump_writer/l...
src/client/linux/minidump_writer/linux_ptrace_dumper.cc:199: if
(sys_ptrace(PTRACE_GETFPXREGS, tid, NULL, &info->fpxregs) == -1) {
On 2013/12/10 03:21:22, Mark Mentovai wrote:
> Another option is to eschew the direct __cpuid stuff here. You could catch a
> ptrace failure with errno set to ENODEV, accept that as legitimate as opposed
to
> failure, and memset the fpxregs field to 0.

Linux kernels older than 2.6.25 return EIO. On the desktop, most users likely
have newer kernels, but you never know in the embedded space. I'd prefer to use
the cpuid, rather than count on ENODEV which my ptrace(2) man page doesn't even
mention.

[Mark Mentovai]

That’s fine with me. LGTM however you want to handle it, cpuid or ptrace error
tolerance.

Note also https://breakpad.appspot.com/824003/ which contained another fix to
this new code.

[Lei Zhang (chromium)]

Committed patchset #2 manually as r1254 (presubmit successful).