win: Fixes for minidump_string_writer_test.cc

win: Fixes for minidump_string_writer_test.cc

- More references to sizeof(Struct::Member)
- Complaints about not being able to generate
  constructor, etc. on initialized struct.

R=mark@chromium.org
BUG=crashpad:1
Committed: https://chromium.googlesource.com/crashpad/crashpad/+/e67b87ef02f44d1d2739caf5eabb849ed117b553

[Mark Mentovai, 2015-02-05 14:37:42]

https://codereview.chromium.org/896423002/diff/1/minidump/minidump_string_wri...
File minidump/minidump_string_writer_test.cc (right):

https://codereview.chromium.org/896423002/diff/1/minidump/minidump_string_wri...
minidump/minidump_string_writer_test.cc:54: #if defined(OS_WIN)
COMPILER_MSVC, not OS_WIN.

Make sure the rest of your MSVC #pragma warnings are COMPILER_MSVC too. I think
they are.

https://codereview.chromium.org/896423002/diff/1/minidump/minidump_string_wri...
minidump/minidump_string_writer_test.cc:56: // Can't generate default
constructor, assignment operator, can never be
This is OK, but I’m surprised to see this kind of warning being generated here.
This pattern for test data shows up in Chromium too, like on line 173 of this
file, and I don’t recall seeing #pragma warning for it ever before. Is the
compiler getting thrown off by the char16 array or is it the const that’s doing
it?

https://codereview.chromium.org/896423002/diff/1/minidump/minidump_string_wri...
minidump/minidump_string_writer_test.cc:101: MINIDUMP_STRING tmp = {0};
(void)tmp;
ALLOW_UNUSED_LOCAL here and on line 145.

[scottmg, 2015-02-05 17:09:39]

https://codereview.chromium.org/896423002/diff/1/minidump/minidump_string_wri...
File minidump/minidump_string_writer_test.cc (right):

https://codereview.chromium.org/896423002/diff/1/minidump/minidump_string_wri...
minidump/minidump_string_writer_test.cc:56: // Can't generate default
constructor, assignment operator, can never be
On 2015/02/05 14:37:42, Mark Mentovai wrote:
> This is OK, but I’m surprised to see this kind of warning being generated
here.
> This pattern for test data shows up in Chromium too, like on line 173 of this
> file, and I don’t recall seeing #pragma warning for it ever before. Is the
> compiler getting thrown off by the char16 array or is it the const that’s
doing
> it?

It's the const. Removing that seems better?

https://codereview.chromium.org/896423002/diff/1/minidump/minidump_string_wri...
minidump/minidump_string_writer_test.cc:101: MINIDUMP_STRING tmp = {0};
(void)tmp;
On 2015/02/05 14:37:42, Mark Mentovai wrote:
> ALLOW_UNUSED_LOCAL here and on line 145.

Done.

[Mark Mentovai, 2015-02-05 17:14:52]

LGTM blah blah DEPS

https://codereview.chromium.org/896423002/diff/20001/minidump/minidump_string...
File minidump/minidump_string_writer_test.cc (right):

https://codereview.chromium.org/896423002/diff/20001/minidump/minidump_string...
minidump/minidump_string_writer_test.cc:59: base::char16 output_string[10];
Yeah, this is better. The data should be const enough because of the “const” on
line 55 anyway.

https://codereview.chromium.org/896423002/diff/20001/minidump/minidump_string...
minidump/minidump_string_writer_test.cc:93: MINIDUMP_STRING tmp = {0};
Is the initialization required? Line 138 too.

[scottmg, 2015-02-05 17:41:58]

https://codereview.chromium.org/896423002/diff/20001/minidump/minidump_string...
File minidump/minidump_string_writer_test.cc (right):

https://codereview.chromium.org/896423002/diff/20001/minidump/minidump_string...
minidump/minidump_string_writer_test.cc:93: MINIDUMP_STRING tmp = {0};
On 2015/02/05 17:14:52, Mark Mentovai wrote:
> Is the initialization required? Line 138 too.

Yes, because of zero-sized array (same as
https://codereview.chromium.org/886143004/).

[scottmg, 2015-02-05 17:42:19]

Committed patchset #2 (id:20001) manually as
e67b87ef02f44d1d2739caf5eabb849ed117b553 (presubmit successful).