minidump_dump: convert time_t values to UTC correctly. Interpret the

minidump_dump: bug fixes.

 - Convert time_t values to UTC correctly. It is incorrect to cast a uint32_t*
   to time_t* because the two types may have different widths. This is the
   case on many 64-bit systems, where time_t is a 64-bit signed integer.
   Conversion is unified in a single function, and additional uses of time_t
   in minidump files not previously displayed in UTC are now displayed.
 - Interpret the IMAGE_DEBUG_MISC structure correctly.
 - When printing MINIDUMP_SYSTEM_INFO structures, always show the "x86" side
   of the union, and state whether it's expected to be valid. (Existing
   Breakpad-produced non-Windows minidumps for x86_64 use the "x86" side of
   union, but Windows minidumps for x86_64 use the "other" side, so I want to
   print both.)

R=ivanpe@chromium.org

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

[Mark Mentovai]

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc
File src/processor/minidump.cc (left):

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc#oldcode...
src/processor/minidump.cc:2904: if (misc_record->unicode)
The old code interpreted the opposite meaning of this flag.

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc
File src/processor/minidump.cc (right):

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc#newcode...
src/processor/minidump.cc:2921: ConvertUTF16BufferToUTF8String(
Might as well do the right thing.

[ivanpe]

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc
File src/processor/minidump.cc (right):

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc#newcode370
src/processor/minidump.cc:370: std::string TimeTToUTCString(time_t tt) {
Instead of std::string, please use just string and include
"common/using_std_string.h"

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc#newcode379
src/processor/minidump.cc:379: strftime(timestr, 20, "%Y-%m-%d %H:%M:%S",
&timestruct);
Maybe you should check return value (on error you should get 0)?  20 chars
should be enough, but still, it is better to check and return empty string or
some error message when the operation fails.

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc#newcode381
src/processor/minidump.cc:381: return std::string(timestr);
same here

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc#newcode...
src/processor/minidump.cc:2920: std::string misc_record_data_utf8;
Same. Please, use string.

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc#newcode...
src/processor/minidump.cc:3879: printf(" 
cpu.other_cpu_info.processor_features[%d]   = 0x%" PRIx64 "\n",
Do you want to print this even in the cases where cpu.other_cpu_info is invalid?

[Mark Mentovai]

Thanks for the review. I’ve addressed the feedback.

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc
File src/processor/minidump.cc (right):

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc#newcode370
src/processor/minidump.cc:370: std::string TimeTToUTCString(time_t tt) {
ivanpe wrote:
> Instead of std::string, please use just string and include
> "common/using_std_string.h"

minidump.h already includes this, so I changed the std::strings to strings.

https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc#newcode...
src/processor/minidump.cc:3879: printf(" 
cpu.other_cpu_info.processor_features[%d]   = 0x%" PRIx64 "\n",
ivanpe wrote:
> Do you want to print this even in the cases where cpu.other_cpu_info is
invalid?

Good point. I want the “x86” side for x86_64, but I don’t need the “other” side
for x86.

[ivanpe]

On 2014/06/17 03:57:16, Mark Mentovai wrote:
> Thanks for the review. I’ve addressed the feedback.
> 
> https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc
> File src/processor/minidump.cc (right):
> 
>
https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc#newcode370
> src/processor/minidump.cc:370: std::string TimeTToUTCString(time_t tt) {
> ivanpe wrote:
> > Instead of std::string, please use just string and include
> > "common/using_std_string.h"
> 
> minidump.h already includes this, so I changed the std::strings to strings.
> 
>
https://breakpad.appspot.com/5674002/diff/1/src/processor/minidump.cc#newcode...
> src/processor/minidump.cc:3879: printf(" 
> cpu.other_cpu_info.processor_features[%d]   = 0x%" PRIx64 "\n",
> ivanpe wrote:
> > Do you want to print this even in the cases where cpu.other_cpu_info is
> invalid?
> 
> Good point. I want the “x86” side for x86_64, but I don’t need the “other”
side
> for x86.

LGTM

[Mark Mentovai]

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