Fix Breakpad unit tests build for Android.

Fix Breakpad unit tests build for Android.

This patch contains the source-related fixes to ensure that
the Breakpad unit tests build properly when targetting Android.

Calling 'make check' still fails because there is still no
way to run the unit test programs on a device. This will be
addressed by a future patch.

Important notes:

- You must target Android API level 9 (Gingerbread) or higher
  to build the unit tests. This requirement is due to the
  current GTest revision used in the breakpad source tree.

- This patch adds headers providing inlined C library functions
  missing from the NDK (see src/common/android/testing). They
  are inlined to avoid modifying Makefile.am and other build
  files.

- Similarly, the <wchar.h> header-fix under
  src/common/android/testing/include is only required by
  GTest's current revision. E.g. it's not needed when building
  Breakpad as part of Chromium which uses a more up-to-date
  revision.
Committed: https://code.google.com/p/google-breakpad/source/detail?r=1020

[digit]

A quick comments about this patch:

- I've decided to use inlined versions of the missing
  Android C library functions. This is in order to
  minimize changes to the build files (i.e. Makefile.am
  or Chromium's breakpad.gyp).

  If you prefer, I can move them into their own support
  library, and modify the build files to link it into
  each one of the unit tests.

- I'm working on upgrading GTest/GMock to a more recent
  version. This is not trivial because this breaks the
  (regular, non-Android) build due to new dependencies
  on pthread and the fact that m4/ax_pthread.m4 doesn't
  set the PTHREAD_XXXX macros properly (or Makefile.am
  isn't using them properly).

  Note that technically, this patch is the only thing
  needed to fix the Android build of the breakpad unit
  tests build for the upstream Chromium tree.

- A future patch will provide a shell wrapper script that
  can be invoked during 'make check' to run the unit tests
  on a device. A lot of tests are failing, but this will
  be addressed later.

Thanks.

[Mark Mentovai]

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/include/...
File src/common/android/testing/include/wchar.h (right):

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/include/...
src/common/android/testing/include/wchar.h:47: static wchar_t __inline__
wcstolower(wchar_t ch) {
Why __inline__ instead of inline? Line 53 too.

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/mkdtemp.h
File src/common/android/testing/mkdtemp.h (right):

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/mkdtemp....
src/common/android/testing/mkdtemp.h:56: if (path_end - path < 6 ||
memcmp(path_end-6, "XXXXXX", 6) != 0) {
All of these sixes (and one seven) are magic. Could you do

  const char kSuffix[] = "XXXXXX";
  const size_t kSuffixLen = strlen(kSuffix);

The compiler is smart enough to optimize a strlen of a constant string away, so
this becomes a compile-time constant tied to the length of the suffix.

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/mkdtemp....
src/common/android/testing/mkdtemp.h:68: ret = stat(path, &st);
Since you’re going to the trouble of doing this stat, you should also check that
the result is in fact a directory, and set ENOTDIR if it isn’t.

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/mkdtemp....
src/common/android/testing/mkdtemp.h:88: errno = EEXIST;
This should already be the case, right? It’s better as an assertion than a
reassignment of the same value it’s already got then.

http://breakpad.appspot.com/439002/diff/1/src/common/linux/tests/crash_genera...
File src/common/linux/tests/crash_generator.cc (right):

http://breakpad.appspot.com/439002/diff/1/src/common/linux/tests/crash_genera...
src/common/linux/tests/crash_generator.cc:55: 
Introducing a little extra blank line here, huh?

http://breakpad.appspot.com/439002/diff/1/src/common/tests/file_utils.cc
File src/common/tests/file_utils.cc (right):

http://breakpad.appspot.com/439002/diff/1/src/common/tests/file_utils.cc#newc...
src/common/tests/file_utils.cc:37: #include <sys/stat.h>
Sort: sys < unistd.

[digit]

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/include/...
File src/common/android/testing/include/wchar.h (right):

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/include/...
src/common/android/testing/include/wchar.h:47: static wchar_t __inline__
wcstolower(wchar_t ch) {
Oh, because I lifted that from a previous C file. Not required here :)

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/mkdtemp.h
File src/common/android/testing/mkdtemp.h (right):

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/mkdtemp....
src/common/android/testing/mkdtemp.h:56: if (path_end - path < 6 ||
memcmp(path_end-6, "XXXXXX", 6) != 0) {
Sure, done.

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/mkdtemp....
src/common/android/testing/mkdtemp.h:68: ret = stat(path, &st);
On 2012/08/22 15:58:55, Mark Mentovai wrote:
> Since you’re going to the trouble of doing this stat, you should also check
that
> the result is in fact a directory, and set ENOTDIR if it isn’t.

Done.

http://breakpad.appspot.com/439002/diff/1/src/common/android/testing/mkdtemp....
src/common/android/testing/mkdtemp.h:88: errno = EEXIST;
On 2012/08/22 15:58:55, Mark Mentovai wrote:
> This should already be the case, right? It’s better as an assertion than a
> reassignment of the same value it’s already got then.

Done.

http://breakpad.appspot.com/439002/diff/1/src/common/linux/tests/crash_genera...
File src/common/linux/tests/crash_generator.cc (right):

http://breakpad.appspot.com/439002/diff/1/src/common/linux/tests/crash_genera...
src/common/linux/tests/crash_generator.cc:55: 
Damned! Line removed.

http://breakpad.appspot.com/439002/diff/1/src/common/tests/file_utils.cc
File src/common/tests/file_utils.cc (right):

http://breakpad.appspot.com/439002/diff/1/src/common/tests/file_utils.cc#newc...
src/common/tests/file_utils.cc:37: #include <sys/stat.h>
On 2012/08/22 15:58:55, Mark Mentovai wrote:
> Sort: sys < unistd.

Done.

[Mark Mentovai]

LGTM