win: Fixes for minidump_string_writer_test.cc

minidump/minidump_string_writer_test.cc

 // Copyright 2014 The Crashpad Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
 //     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
@@ skipping 33 matching lines @@
     EXPECT_TRUE(string_writer.WriteEverything(&file_writer));
     ASSERT_EQ(6u, file_writer.string().size());
 
     const MINIDUMP_STRING* minidump_string =
         MinidumpStringAtRVA(file_writer.string(), 0);
     EXPECT_TRUE(minidump_string);
     EXPECT_EQ(base::string16(),
               MinidumpStringAtRVAAsString(file_writer.string(), 0));
   }
 
+#if defined(OS_WIN)

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

COMPILER_MSVC, not OS_WIN.

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

+#pragma warning(push)
+// Can't generate default constructor, assignment operator, can never be

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

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?

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

It's the const. Removing that seems better?

+// instantiated.
+#pragma warning(disable: 4510 4512 4610)
+#endif  // OS_WIN
   const struct {
     size_t input_length;
     const char* input_string;
     size_t output_length;
     const base::char16 output_string[10];
   } kTestData[] = {
       {0, "", 0, {}},
       {1, "a", 1, {'a'}},
       {2, "\0b", 2, {0, 'b'}},
       {3, "cde", 3, {'c', 'd', 'e'}},
       {9, "Hi world!", 9, {'H', 'i', ' ', 'w', 'o', 'r', 'l', 'd', '!'}},
       {7, "ret\nurn", 7, {'r', 'e', 't', '\n', 'u', 'r', 'n'}},
       {2, "\303\251", 1, {0x00e9}},  // é
 
       // oóöőo
       {8, "o\303\263\303\266\305\221o", 5, {'o', 0x00f3, 0x00f6, 0x151, 'o'}},
       {4, "\360\220\204\202", 2, {0xd800, 0xdd02}},  // 𐄂 (non-BMP)
   };
+#if defined(OS_WIN)
+#pragma warning(pop)
+#endif  // OS_WIN
 
   for (size_t index = 0; index < arraysize(kTestData); ++index) {
     SCOPED_TRACE(base::StringPrintf(
         "index %zu, input %s", index, kTestData[index].input_string));
 
     // Make sure that the expected output string with its NUL terminator fits in
     // the space provided.
     ASSERT_EQ(
         0,
         kTestData[index]
             .output_string[arraysize(kTestData[index].output_string) - 1]);
 
     file_writer.Reset();
     crashpad::internal::MinidumpUTF16StringWriter string_writer;
     string_writer.SetUTF8(std::string(kTestData[index].input_string,
                                       kTestData[index].input_length));
     EXPECT_TRUE(string_writer.WriteEverything(&file_writer));
 
     const size_t expected_utf16_units_with_nul =
         kTestData[index].output_length + 1;
+    MINIDUMP_STRING tmp = {0}; (void)tmp;

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

ALLOW_UNUSED_LOCAL here and on line 145.

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

Done.

     const size_t expected_utf16_bytes =
-        expected_utf16_units_with_nul * sizeof(MINIDUMP_STRING::Buffer[0]);
+        expected_utf16_units_with_nul * sizeof(tmp.Buffer[0]);
     ASSERT_EQ(sizeof(MINIDUMP_STRING) + expected_utf16_bytes,
               file_writer.string().size());
 
     const MINIDUMP_STRING* minidump_string =
         MinidumpStringAtRVA(file_writer.string(), 0);
     EXPECT_TRUE(minidump_string);
     base::string16 expect_string = base::string16(
         kTestData[index].output_string, kTestData[index].output_length);
     EXPECT_EQ(expect_string,
               MinidumpStringAtRVAAsString(file_writer.string(), 0));
@@ skipping 21 matching lines @@
     EXPECT_TRUE(string_writer.WriteEverything(&file_writer));
 
     // The requirements for conversion of invalid UTF-8 input are lax. Make sure
     // that at least enough data was written for a string that has one unit and
     // a NUL terminator, make sure that the length field matches the length of
     // data written, and make sure that at least one U+FFFD replacement
     // character was written.
     const MINIDUMP_STRING* minidump_string =
         MinidumpStringAtRVA(file_writer.string(), 0);
     EXPECT_TRUE(minidump_string);
+    MINIDUMP_STRING tmp = {0}; (void)tmp;
     EXPECT_EQ(file_writer.string().size() - sizeof(MINIDUMP_STRING) -
-                  sizeof(MINIDUMP_STRING::Buffer[0]),
+                  sizeof(tmp.Buffer[0]),
               minidump_string->Length);
     base::string16 output_string =
         MinidumpStringAtRVAAsString(file_writer.string(), 0);
     EXPECT_FALSE(output_string.empty());
     EXPECT_NE(base::string16::npos, output_string.find(0xfffd));
   }
 }
 
 TEST(MinidumpStringWriter, MinidumpUTF8StringWriter) {
   StringFileWriter file_writer;
@@ skipping 106 matching lines @@
   MinidumpStringListTest<MinidumpUTF16StringListWriterTraits>();
 }
 
 TEST(MinidumpStringWriter, MinidumpUTF8StringList) {
   MinidumpStringListTest<MinidumpUTF8StringListWriterTraits>();
 }
 
 }  // namespace
 }  // namespace test
 }  // namespace crashpad