Fixing various compiler warnings and applying minor tweaks to allow running of

the mojority of breakpad unittests in Google.

http://breakpad.appspot.com/399002/



git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@978 4c0a9323-5329-0410-9bdc-e9ce6186880e

Makefile.am

@@ -33,6 +33,16 @@
 # This allows #includes to be relative to src/
 AM_CPPFLAGS = -I$(top_srcdir)/src
 
+if GCC
+# These are good warnings to be treated as errors
+AM_CXXFLAGS = \
+	-Werror=non-virtual-dtor \
+	-Werror=vla \
+	-Werror=unused-variable \
+	-Werror=missing-braces \
+	-Werror=overloaded-virtual
+endif
+
 # Specify include paths for ac macros
 ACLOCAL_AMFLAGS = -I m4
 
@@ -294,6 +304,7 @@ src_client_linux_linux_client_unittest_SOURCES = \
 	src/client/linux/minidump_writer/linux_core_dumper_unittest.cc \
 	src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc \
 	src/client/linux/minidump_writer/minidump_writer_unittest.cc \
+	src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc \
 	src/common/linux/linux_libc_support_unittest.cc \
 	src/common/linux/tests/crash_generator.cc \
 	src/common/memory_unittest.cc \

Makefile.in

@@ -395,6 +395,7 @@ am__src_client_linux_linux_client_unittest_SOURCES_DIST =  \
 	src/client/linux/minidump_writer/linux_core_dumper_unittest.cc \
 	src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc \
 	src/client/linux/minidump_writer/minidump_writer_unittest.cc \
+	src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc \
 	src/common/linux/linux_libc_support_unittest.cc \
 	src/common/linux/tests/crash_generator.cc \
 	src/common/memory_unittest.cc src/common/tests/file_utils.cc \
@@ -409,6 +410,7 @@ am__src_client_linux_linux_client_unittest_SOURCES_DIST =  \
 @LINUX_HOST_TRUE@	src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-linux_core_dumper_unittest.$(OBJEXT) \
 @LINUX_HOST_TRUE@	src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-linux_ptrace_dumper_unittest.$(OBJEXT) \
 @LINUX_HOST_TRUE@	src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest.$(OBJEXT) \
+@LINUX_HOST_TRUE@	src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.$(OBJEXT) \
 @LINUX_HOST_TRUE@	src/common/linux/src_client_linux_linux_client_unittest-linux_libc_support_unittest.$(OBJEXT) \
 @LINUX_HOST_TRUE@	src/common/linux/tests/src_client_linux_linux_client_unittest-crash_generator.$(OBJEXT) \
 @LINUX_HOST_TRUE@	src/common/src_client_linux_linux_client_unittest-memory_unittest.$(OBJEXT) \
@@ -1203,6 +1205,15 @@ top_srcdir = @top_srcdir@
 # This allows #includes to be relative to src/
 AM_CPPFLAGS = -I$(top_srcdir)/src
 
+# These are good warnings to be treated as errors
+@GCC_TRUE@AM_CXXFLAGS = \
+@GCC_TRUE@	-Werror=non-virtual-dtor \
+@GCC_TRUE@	-Werror=vla \
+@GCC_TRUE@	-Werror=unused-variable \
+@GCC_TRUE@	-Werror=missing-braces \
+@GCC_TRUE@	-Werror=overloaded-virtual
+
+
 # Specify include paths for ac macros
 ACLOCAL_AMFLAGS = -I m4
 dist_doc_DATA = \
@@ -1373,6 +1384,7 @@ TESTS_ENVIRONMENT =
 @LINUX_HOST_TRUE@	src/client/linux/minidump_writer/linux_core_dumper_unittest.cc \
 @LINUX_HOST_TRUE@	src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc \
 @LINUX_HOST_TRUE@	src/client/linux/minidump_writer/minidump_writer_unittest.cc \
+@LINUX_HOST_TRUE@	src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc \
 @LINUX_HOST_TRUE@	src/common/linux/linux_libc_support_unittest.cc \
 @LINUX_HOST_TRUE@	src/common/linux/tests/crash_generator.cc \
 @LINUX_HOST_TRUE@	src/common/memory_unittest.cc \
@@ -2466,6 +2478,9 @@ src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-linux_pt
 src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest.$(OBJEXT):  \
 	src/client/linux/minidump_writer/$(am__dirstamp) \
 	src/client/linux/minidump_writer/$(DEPDIR)/$(am__dirstamp)
+src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.$(OBJEXT):  \
+	src/client/linux/minidump_writer/$(am__dirstamp) \
+	src/client/linux/minidump_writer/$(DEPDIR)/$(am__dirstamp)
 src/common/linux/src_client_linux_linux_client_unittest-linux_libc_support_unittest.$(OBJEXT):  \
 	src/common/linux/$(am__dirstamp) \
 	src/common/linux/$(DEPDIR)/$(am__dirstamp)
@@ -3107,6 +3122,7 @@ mostlyclean-compile:
 	-rm -f src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-linux_core_dumper_unittest.$(OBJEXT)
 	-rm -f src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-linux_ptrace_dumper_unittest.$(OBJEXT)
 	-rm -f src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest.$(OBJEXT)
+	-rm -f src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.$(OBJEXT)
 	-rm -f src/client/linux/minidump_writer/src_client_linux_linux_dumper_unittest_helper-linux_dumper_unittest_helper.$(OBJEXT)
 	-rm -f src/client/minidump_file_writer.$(OBJEXT)
 	-rm -f src/common/convert_UTF.$(OBJEXT)
@@ -3329,6 +3345,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-linux_core_dumper_unittest.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-linux_ptrace_dumper_unittest.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest.Po@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_dumper_unittest_helper-linux_dumper_unittest_helper.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@src/common/$(DEPDIR)/convert_UTF.Po@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@src/common/$(DEPDIR)/dwarf_cfi_to_module.Po@am__quote@
@@ -3649,6 +3666,20 @@ src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump
 @AMDEP_TRUE@@am__fastdepCXX_FALSE@	DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@
 @am__fastdepCXX_FALSE@	$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -c -o src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest.obj `if test -f 'src/client/linux/minidump_writer/minidump_writer_unittest.cc'; then $(CYGPATH_W) 'src/client/linux/minidump_writer/minidump_writer_unittest.cc'; else $(CYGPATH_W) '$(srcdir)/src/client/linux/minidump_writer/minidump_writer_unittest.cc'; fi`
 
+src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.o: src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc
+@am__fastdepCXX_TRUE@	$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -MT src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.o -MD -MP -MF src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Tpo -c -o src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.o `test -f 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc' || echo '$(srcdir)/'`src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc
+@am__fastdepCXX_TRUE@	$(am__mv) src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Tpo src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Po
+@AMDEP_TRUE@@am__fastdepCXX_FALSE@	source='src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc' object='src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.o' libtool=no @AMDEPBACKSLASH@
+@AMDEP_TRUE@@am__fastdepCXX_FALSE@	DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@
+@am__fastdepCXX_FALSE@	$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -c -o src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.o `test -f 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc' || echo '$(srcdir)/'`src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc
+
+src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.obj: src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc
+@am__fastdepCXX_TRUE@	$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -MT src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.obj -MD -MP -MF src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Tpo -c -o src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.obj `if test -f 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; then $(CYGPATH_W) 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; else $(CYGPATH_W) '$(srcdir)/src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; fi`
+@am__fastdepCXX_TRUE@	$(am__mv) src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Tpo src/client/linux/minidump_writer/$(DEPDIR)/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.Po
+@AMDEP_TRUE@@am__fastdepCXX_FALSE@	source='src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc' object='src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.obj' libtool=no @AMDEPBACKSLASH@
+@AMDEP_TRUE@@am__fastdepCXX_FALSE@	DEPDIR=$(DEPDIR) $(CXXDEPMODE) $(depcomp) @AMDEPBACKSLASH@
+@am__fastdepCXX_FALSE@	$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -c -o src/client/linux/minidump_writer/src_client_linux_linux_client_unittest-minidump_writer_unittest_utils.obj `if test -f 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; then $(CYGPATH_W) 'src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; else $(CYGPATH_W) '$(srcdir)/src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc'; fi`
+
 src/common/linux/src_client_linux_linux_client_unittest-linux_libc_support_unittest.o: src/common/linux/linux_libc_support_unittest.cc
 @am__fastdepCXX_TRUE@	$(CXX) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(src_client_linux_linux_client_unittest_CPPFLAGS) $(CPPFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS) -MT src/common/linux/src_client_linux_linux_client_unittest-linux_libc_support_unittest.o -MD -MP -MF src/common/linux/$(DEPDIR)/src_client_linux_linux_client_unittest-linux_libc_support_unittest.Tpo -c -o src/common/linux/src_client_linux_linux_client_unittest-linux_libc_support_unittest.o `test -f 'src/common/linux/linux_libc_support_unittest.cc' || echo '$(srcdir)/'`src/common/linux/linux_libc_support_unittest.cc
 @am__fastdepCXX_TRUE@	$(am__mv) src/common/linux/$(DEPDIR)/src_client_linux_linux_client_unittest-linux_libc_support_unittest.Tpo src/common/linux/$(DEPDIR)/src_client_linux_linux_client_unittest-linux_libc_support_unittest.Po

configure

@@ -612,6 +612,8 @@ PTHREAD_CC
 ax_pthread_config
 EGREP
 GREP
+GCC_FALSE
+GCC_TRUE
 RANLIB
 am__fastdepCXX_FALSE
 am__fastdepCXX_TRUE
@@ -4498,6 +4500,14 @@ else
   RANLIB="$ac_cv_prog_RANLIB"
 fi
 
+ if test "$GCC" = yes; then
+  GCC_TRUE=
+  GCC_FALSE='#'
+else
+  GCC_TRUE='#'
+  GCC_FALSE=
+fi
+   # let the Makefile know if we're gcc
 
 
 
@@ -5463,6 +5473,10 @@ if test -z "${am__fastdepCXX_TRUE}" && test -z "${am__fastdepCXX_FALSE}"; then
   as_fn_error "conditional \"am__fastdepCXX\" was never defined.
 Usually this means the macro was only invoked conditionally." "$LINENO" 5
 fi
+if test -z "${GCC_TRUE}" && test -z "${GCC_FALSE}"; then
+  as_fn_error "conditional \"GCC\" was never defined.
+Usually this means the macro was only invoked conditionally." "$LINENO" 5
+fi
 if test -z "${LINUX_HOST_TRUE}" && test -z "${LINUX_HOST_FALSE}"; then
   as_fn_error "conditional \"LINUX_HOST\" was never defined.
 Usually this means the macro was only invoked conditionally." "$LINENO" 5

configure.ac

@@ -45,6 +45,7 @@ AM_PROG_CC_C_O
 AC_PROG_CPP
 AC_PROG_CXX
 AC_PROG_RANLIB
+AM_CONDITIONAL(GCC, test "$GCC" = yes)   # let the Makefile know if we're gcc
 
 AC_HEADER_STDC
 m4_include(m4/ax_pthread.m4)

src/client/linux/minidump_writer/linux_core_dumper_unittest.cc

@@ -83,11 +83,10 @@ TEST(LinuxCoreDumperTest, VerifyDumpWithMultipleThreads) {
     return;
   }
 
-  pid_t pid = getpid();
   const string core_file = crash_generator.GetCoreFilePath();
   const string procfs_path = crash_generator.GetDirectoryOfProcFilesCopy();
   LinuxCoreDumper dumper(child_pid, core_file.c_str(), procfs_path.c_str());
-  dumper.Init();
+  EXPECT_TRUE(dumper.Init());
 
   EXPECT_TRUE(dumper.IsPostMortem());
 

src/client/linux/minidump_writer/linux_dumper_unittest_helper.cc

@@ -38,6 +38,7 @@
 #include <sys/syscall.h>
 #include <unistd.h>
 
+#include "processor/scoped_ptr.h"
 #include "third_party/lss/linux_syscall_support.h"
 
 #if defined(__ARM_EABI__)
@@ -77,7 +78,7 @@ int main(int argc, char *argv[]) {
     fprintf(stderr, "ERROR: number of threads is 0");
     return 1;
   }
-  pthread_t threads[num_threads];
+  google_breakpad::scoped_array<pthread_t> threads(new pthread_t[num_threads]);
   pthread_attr_t thread_attributes;
   pthread_attr_init(&thread_attributes);
   pthread_attr_setdetachstate(&thread_attributes, PTHREAD_CREATE_DETACHED);

src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc

@@ -33,11 +33,13 @@
 // This file was renamed from linux_dumper_unittest.cc and modified due
 // to LinuxDumper being splitted into two classes.
 
+#include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
 #include <unistd.h>
 #include <signal.h>
 #include <stdint.h>
+#include <string.h>
 #include <sys/mman.h>
 #include <sys/poll.h>
 #include <sys/stat.h>
@@ -47,6 +49,7 @@
 
 #include "breakpad_googletest_includes.h"
 #include "client/linux/minidump_writer/linux_ptrace_dumper.h"
+#include "client/linux/minidump_writer/minidump_writer_unittest_utils.h"
 #include "common/linux/eintr_wrapper.h"
 #include "common/linux/file_id.h"
 #include "common/linux/safe_readlink.h"
@@ -59,23 +62,6 @@ namespace {
 
 typedef testing::Test LinuxPtraceDumperTest;
 
-string GetHelperBinary() {
-  // Locate helper binary next to the current binary.
-  char self_path[PATH_MAX];
-  if (!SafeReadLink("/proc/self/exe", self_path)) {
-    return "";
-  }
-  string helper_path(self_path);
-  size_t pos = helper_path.rfind('/');
-  if (pos == string::npos) {
-    return "";
-  }
-  helper_path.erase(pos + 1);
-  helper_path += "linux_dumper_unittest_helper";
-
-  return helper_path;
-}
-
 }  // namespace
 
 TEST(LinuxPtraceDumperTest, Setup) {
@@ -134,7 +120,8 @@ TEST(LinuxPtraceDumperTest, MergedMappings) {
   const size_t kPageSize = sysconf(_SC_PAGESIZE);
   const size_t kMappingSize = 3 * kPageSize;
   int fd = open(helper_path.c_str(), O_RDONLY);
-  ASSERT_NE(-1, fd);
+  ASSERT_NE(-1, fd) << "Failed to open file: " << helper_path
+                    << ", Error: " << strerror(errno);
   char* mapping =
     reinterpret_cast<char*>(mmap(NULL,
                                  kMappingSize,

src/client/linux/minidump_writer/minidump_writer_unittest.cc

@@ -40,12 +40,14 @@
 #include "client/linux/handler/exception_handler.h"
 #include "client/linux/minidump_writer/linux_dumper.h"
 #include "client/linux/minidump_writer/minidump_writer.h"
+#include "client/linux/minidump_writer/minidump_writer_unittest_utils.h"
 #include "common/linux/eintr_wrapper.h"
 #include "common/linux/file_id.h"
 #include "common/linux/safe_readlink.h"
 #include "common/tests/auto_tempdir.h"
 #include "common/using_std_string.h"
 #include "google_breakpad/processor/minidump.h"
+#include "processor/scoped_ptr.h"
 
 using namespace google_breakpad;
 
@@ -94,7 +96,7 @@ TEST(MinidumpWriterTest, MappingInfo) {
 
   // These are defined here so the parent can use them to check the
   // data from the minidump afterwards.
-  const u_int32_t kMemorySize = sysconf(_SC_PAGESIZE);
+  const u_int32_t memory_size = sysconf(_SC_PAGESIZE);
   const char* kMemoryName = "a fake module";
   const u_int8_t kModuleGUID[sizeof(MDGUID)] = {
     0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
@@ -117,7 +119,7 @@ TEST(MinidumpWriterTest, MappingInfo) {
   // Get some memory.
   char* memory =
     reinterpret_cast<char*>(mmap(NULL,
-                                 kMemorySize,
+                                 memory_size,
                                  PROT_READ | PROT_WRITE,
                                  MAP_PRIVATE | MAP_ANON,
                                  -1,
@@ -145,7 +147,7 @@ TEST(MinidumpWriterTest, MappingInfo) {
   // Add information about the mapped memory.
   MappingInfo info;
   info.start_addr = kMemoryAddress;
-  info.size = kMemorySize;
+  info.size = memory_size;
   info.offset = 0;
   strcpy(info.name, kMemoryName);
 
@@ -170,7 +172,7 @@ TEST(MinidumpWriterTest, MappingInfo) {
   ASSERT_TRUE(module);
 
   EXPECT_EQ(kMemoryAddress, module->base_address());
-  EXPECT_EQ(kMemorySize, module->size());
+  EXPECT_EQ(memory_size, module->size());
   EXPECT_EQ(kMemoryName, module->code_file());
   EXPECT_EQ(module_identifier, module->debug_identifier());
 
@@ -186,7 +188,7 @@ TEST(MinidumpWriterTest, MappingInfoContained) {
 
   // These are defined here so the parent can use them to check the
   // data from the minidump afterwards.
-  const u_int32_t kMemorySize = sysconf(_SC_PAGESIZE);
+  const u_int32_t memory_size = sysconf(_SC_PAGESIZE);
   const char* kMemoryName = "a fake module";
   const u_int8_t kModuleGUID[sizeof(MDGUID)] = {
     0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
@@ -213,14 +215,14 @@ TEST(MinidumpWriterTest, MappingInfoContained) {
   ASSERT_NE(-1, fd);
   unlink(tempfile.c_str());
   // fill with zeros
-  char buffer[kMemorySize];
+  google_breakpad::scoped_array<char> buffer(new char[memory_size]);
-  memset(buffer, 0, kMemorySize);
+  memset(buffer.get(), 0, memory_size);
-  ASSERT_EQ(kMemorySize, write(fd, buffer, kMemorySize));
+  ASSERT_EQ(memory_size, write(fd, buffer.get(), memory_size));
   lseek(fd, 0, SEEK_SET);
 
   char* memory =
     reinterpret_cast<char*>(mmap(NULL,
-                                 kMemorySize,
+                                 memory_size,
                                  PROT_READ | PROT_WRITE,
                                  MAP_PRIVATE,
                                  fd,
@@ -248,8 +250,8 @@ TEST(MinidumpWriterTest, MappingInfoContained) {
   // Add information about the mapped memory. Report it as being larger than
   // it actually is.
   MappingInfo info;
-  info.start_addr = kMemoryAddress - kMemorySize;
+  info.start_addr = kMemoryAddress - memory_size;
-  info.size = kMemorySize * 3;
+  info.size = memory_size * 3;
   info.offset = 0;
   strcpy(info.name, kMemoryName);
 
@@ -287,20 +289,11 @@ TEST(MinidumpWriterTest, DeletedBinary) {
   char kNumberOfThreadsArgument[2];
   sprintf(kNumberOfThreadsArgument, "%d", kNumberOfThreadsInHelperProgram);
 
-  // Locate helper binary next to the current binary.
+  string helper_path(GetHelperBinary());
-  char self_path[PATH_MAX];
+  if (helper_path.empty()) {
-  if (!SafeReadLink("/proc/self/exe", self_path)) {
+    FAIL() << "Couldn't find helper binary";
-    FAIL() << "readlink failed";
     exit(1);
   }
-  string helper_path(self_path);
-  size_t pos = helper_path.rfind('/');
-  if (pos == string::npos) {
-    FAIL() << "no trailing slash in path: " << helper_path;
-    exit(1);
-  }
-  helper_path.erase(pos + 1);
-  helper_path += "linux_dumper_unittest_helper";
 
   // Copy binary to a temp file.
   AutoTempDir temp_dir;
@@ -308,7 +301,7 @@ TEST(MinidumpWriterTest, DeletedBinary) {
   char cmdline[2 * PATH_MAX];
   sprintf(cmdline, "/bin/cp \"%s\" \"%s\"", helper_path.c_str(),
           binpath.c_str());
-  ASSERT_EQ(0, system(cmdline));
+  ASSERT_EQ(0, system(cmdline)) << "Failed to execute: " << cmdline;
   ASSERT_EQ(0, chmod(binpath.c_str(), 0755));
 
   int fds[2];
@@ -381,6 +374,7 @@ TEST(MinidumpWriterTest, DeletedBinary) {
                                     kGUIDStringSize);
   string module_identifier(identifier_string);
   // Strip out dashes
+  size_t pos;
   while ((pos = module_identifier.find('-')) != string::npos) {
     module_identifier.erase(pos, 1);
   }

src/client/linux/minidump_writer/minidump_writer_unittest_utils.cc

@@ -0,0 +1,66 @@
+// Copyright (c) 2011 Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// minidump_writer_unittest_utils.cc:
+// Shared routines used by unittests under client/linux/minidump_writer.
+
+#include <limits.h>
+#include <stdlib.h>
+
+#include "client/linux/minidump_writer/minidump_writer_unittest_utils.h"
+#include "common/linux/safe_readlink.h"
+#include "common/using_std_string.h"
+
+namespace google_breakpad {
+
+string GetHelperBinary() {
+  string helper_path;
+  char *bindir = getenv("bindir");
+  if (bindir) {
+    helper_path = string(bindir) + "/";
+  } else {
+    // Locate helper binary next to the current binary.
+    char self_path[PATH_MAX];
+    if (!SafeReadLink("/proc/self/exe", self_path)) {
+      return "";
+    }
+    helper_path = string(self_path);
+    size_t pos = helper_path.rfind('/');
+    if (pos == string::npos) {
+      return "";
+    }
+    helper_path.erase(pos + 1);
+  }
+
+  helper_path += "linux_dumper_unittest_helper";
+
+  return helper_path;
+}
+
+}  // namespace google_breakpad

src/client/linux/minidump_writer/minidump_writer_unittest_utils.h

@@ -0,0 +1,49 @@
+// Copyright (c) 2012, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// minidump_writer_unittest_utils.h:
+// Shared routines used by unittests under client/linux/minidump_writer.
+
+#ifndef CLIENT_LINUX_MINIDUMP_WRITER_MINIDUMP_WRITER_UNITTEST_UTILS_H_
+#define CLIENT_LINUX_MINIDUMP_WRITER_MINIDUMP_WRITER_UNITTEST_UTILS_H_
+
+#include <string>
+
+#include "common/using_std_string.h"
+
+namespace google_breakpad {
+
+// Returns the full path to linux_dumper_unittest_helper.  The full path is
+// discovered either by using the environment variable "bindir" or by using
+// the location of the main module of the currently running process.
+string GetHelperBinary();
+
+}  // namespace google_breakpad
+
+#endif  // CLIENT_LINUX_MINIDUMP_WRITER_MINIDUMP_WRITER_UNITTEST_UTILS_H_

src/common/basictypes.h

@@ -32,8 +32,10 @@
 
 // A macro to disallow the copy constructor and operator= functions
 // This should be used in the private: declarations for a class
+#ifndef DISALLOW_COPY_AND_ASSIGN
 #define DISALLOW_COPY_AND_ASSIGN(TypeName) \
   TypeName(const TypeName&);               \
   void operator=(const TypeName&)
+#endif  // DISALLOW_COPY_AND_ASSIGN
 
 #endif  // COMMON_BASICTYPES_H_

src/common/language.h

@@ -50,6 +50,10 @@ namespace google_breakpad {
 // language.
 class Language {
  public:
+  // A base class destructor should be either public and virtual,
+  // or protected and nonvirtual.
+  virtual ~Language() {}
+
   // Return true if this language has functions to which we can assign
   // line numbers. (Debugging info for assembly language, for example,
   // can have source location information, but does not have functions

src/common/linux/elf_core_dump_unittest.cc

@@ -183,7 +183,9 @@ TEST(ElfCoreDumpTest, ValidCoreFile) {
   size_t num_nt_prpsinfo = 0;
   size_t num_nt_prstatus = 0;
   size_t num_nt_fpregset = 0;
+#if defined(__i386__)
   size_t num_nt_prxfpreg = 0;
+#endif
   set<pid_t> actual_thread_ids;
   ElfCoreDump::Note note = core.GetFirstNote();
   while (note.IsValid()) {
@@ -211,7 +213,7 @@ TEST(ElfCoreDumpTest, ValidCoreFile) {
         ++num_nt_prstatus;
         break;
       }
-#if defined(__i386) || defined(__x86_64)
+#if defined(__i386__) || defined(__x86_64__)
       case NT_FPREGSET: {
         EXPECT_TRUE(description.data() != NULL);
         EXPECT_EQ(sizeof(user_fpregs_struct), description.length());
@@ -219,7 +221,7 @@ TEST(ElfCoreDumpTest, ValidCoreFile) {
         break;
       }
 #endif
-#if defined(__i386)
+#if defined(__i386__)
       case NT_PRXFPREG: {
         EXPECT_TRUE(description.data() != NULL);
         EXPECT_EQ(sizeof(user_fpxregs_struct), description.length());
@@ -236,10 +238,10 @@ TEST(ElfCoreDumpTest, ValidCoreFile) {
   EXPECT_TRUE(expected_thread_ids == actual_thread_ids);
   EXPECT_EQ(1, num_nt_prpsinfo);
   EXPECT_EQ(kNumOfThreads, num_nt_prstatus);
-#if defined(__i386) || defined(__x86_64)
+#if defined(__i386__) || defined(__x86_64__)
   EXPECT_EQ(kNumOfThreads, num_nt_fpregset);
 #endif
-#if defined(__i386)
+#if defined(__i386__)
   EXPECT_EQ(kNumOfThreads, num_nt_prxfpreg);
 #endif
 }

src/common/linux/file_id_unittest.cc

@@ -73,9 +73,11 @@ TEST(FileIDStripTest, StripSelf) {
   string templ = temp_dir.path() + "/file-id-unittest";
   char cmdline[4096];
   sprintf(cmdline, "cp \"%s\" \"%s\"", exe_name, templ.c_str());
-  ASSERT_EQ(system(cmdline), 0);
+  ASSERT_EQ(0, system(cmdline)) << "Failed to execute: " << cmdline;
+  sprintf(cmdline, "chmod u+w \"%s\"", templ.c_str());
+  ASSERT_EQ(0, system(cmdline)) << "Failed to execute: " << cmdline;
   sprintf(cmdline, "strip \"%s\"", templ.c_str());
-  ASSERT_EQ(system(cmdline), 0);
+  ASSERT_EQ(0, system(cmdline)) << "Failed to execute: " << cmdline;
 
   uint8_t identifier1[sizeof(MDGUID)];
   uint8_t identifier2[sizeof(MDGUID)];

src/common/linux/linux_libc_support_unittest.cc

@@ -27,8 +27,8 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+#include "breakpad_googletest_includes.h"
 #include "common/linux/linux_libc_support.h"
-#include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 typedef testing::Test LinuxLibcSupportTest;

src/common/memory_unittest.cc

@@ -27,8 +27,8 @@
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+#include "breakpad_googletest_includes.h"
 #include "common/memory.h"
-#include "testing/gtest/include/gtest/gtest.h"
 
 using namespace google_breakpad;
 

src/common/test_assembler.h

@@ -271,7 +271,10 @@ class Section {
  public:
   Section(Endianness endianness = kUnsetEndian)
       : endianness_(endianness) { };
-  ~Section() { };
+
+  // A base class destructor should be either public and virtual,
+  // or protected and nonvirtual.
+  virtual ~Section() { };
 
   // Set the default endianness of this section to ENDIANNESS. This
   // sets the behavior of the D<N> appending functions. If the

src/processor/synth_minidump_unittest.cc

@@ -51,7 +51,6 @@ using google_breakpad::SynthMinidump::Section;
 using google_breakpad::SynthMinidump::Stream;
 using google_breakpad::SynthMinidump::String;
 using google_breakpad::SynthMinidump::SystemInfo;
-using google_breakpad::SynthMinidump::Thread;
 using google_breakpad::test_assembler::kBigEndian;
 using google_breakpad::test_assembler::kLittleEndian;
 using google_breakpad::test_assembler::Label;
@@ -168,7 +167,8 @@ TEST(Thread, Simple) {
   Memory stack(dump, 0xaad55a93cc3c0efcULL);
   stack.Append("stack contents");
   stack.Finish(0xe08cdbd1);
-  Thread thread(dump, 0x3d7ec360, stack, context,
+  google_breakpad::SynthMinidump::Thread thread(
+      dump, 0x3d7ec360, stack, context,
       0x3593f44d, // suspend count
       0xab352b82, // priority class
       0x2753d838, // priority

src/processor/synth_minidump_unittest_data.h

@@ -247,6 +247,7 @@ static const MDRawContextARM arm_raw_context = {
   // context_flags
   0x591b9e6a,
   // iregs
+  {
     0xa21594de,
     0x820d8a25,
     0xc4e133b2,
@@ -263,6 +264,7 @@ static const MDRawContextARM arm_raw_context = {
     0x20988bcb,
     0xf1ab806b,
     0x99d5fc03,
+  },
   // cpsr
   0xb70df511,
   // float_save
@@ -270,6 +272,7 @@ static const MDRawContextARM arm_raw_context = {
     // fpscr
     0xa1e1f7ce1077e6b5ULL,
     // regs
+    {
       0xbcb8d002eed7fbdeULL,
       0x4dd26a43b96ae97fULL,
       0x8eec22db8b31741cULL,
@@ -302,7 +305,9 @@ static const MDRawContextARM arm_raw_context = {
       0x5ceca87836fe2251ULL,
       0x66f50de463ee238cULL,
       0x42823efcd59ab511ULL,
+    },
     // extra
+    {
       0xe9e14cd2,
       0x865bb640,
       0x9f3f0b3e,
@@ -310,7 +315,8 @@ static const MDRawContextARM arm_raw_context = {
       0x3c012f19,
       0x6436637c,
       0x46ccedcb,
-    0x7b341be7
+      0x7b341be7,
+    }
   }
 };
 

src/tools/linux/md2core/minidump-2-core.cc

@@ -49,10 +49,10 @@
 #include "client/linux/minidump_writer/minidump_extension_linux.h"
 #include "common/linux/memory_mapped_file.h"
 #include "google_breakpad/common/minidump_format.h"
+#include "processor/scoped_ptr.h"
 #include "third_party/lss/linux_syscall_support.h"
 #include "tools/linux/md2core/minidump_memory_range.h"
 
-
 #if __WORDSIZE == 64
   #define ELF_CLASS ELFCLASS64
 #else
@@ -1100,9 +1100,9 @@ main(int argc, char** argv) {
   }
 
   if (note_align) {
-    char scratch[note_align];
+    google_breakpad::scoped_array<char> scratch(new char[note_align]);
-    memset(scratch, 0, sizeof(scratch));
+    memset(scratch.get(), 0, note_align);
-    if (!writea(1, scratch, sizeof(scratch)))
+    if (!writea(1, scratch.get(), note_align))
       return 1;
   }