Remove some linux libc calls from client library.

Remove some linux libc calls from client library.

This patch removes a few libc calls from the client library
when generating a minidump.

+ Move the static inlined functions from linux_libc_support.h
  into their own source file to avoid code duplication.

Tested on linux-x86_64 with 'make check' and 'android/run-checks.sh'
Committed: https://code.google.com/p/google-breakpad/source/detail?r=1001

[digit]

A few remarks about this patch:

- I tried to follow the coding style of each original file.
  They are often inconsistent, like the placement of the '*'
  for pointer types and casts in libc_support.h. or the use
  of C-style casts instead of C++ ones. Feel free to correct
  me.

- The original sources seem to assume that memcpy()/memset()
  are safe in the context of a compromised environment, but
  I think it's better to use my_memcpy() / my_memset() in
  all user code, to make it easier to audit.

  That doesn't mean that we can't change my_memcpy and
  my_memmove to simple macro aliases to memcpy/memmove
  (or even __builtin_memcpy / __builtin_memmove when that
  makes sense).

- I'm not sure the comment about a the code running in a
  compromised environment for linux_ptrace_dumper.cc is
  valid. My understanding is that only the attached process
  could be compromised. However, I "fixed" the source file
  just in case.

  I didn't touch "linux_core_dumper.cc" though.

- my_strrchr will be used in a future ARM-specific patch.
  Its usefulness doesn't appear yet.

[Mark Mentovai]

I don’t know how I feel about all of these inlines.

Except for the strl family and the locale-insensitive isspace, can we make the
default be to just call the normal libc functions, which affords the compiler
the opportunity to use the __builtin_ forms?

Because the strl family doesn’t exist in glibc, and the locale-insensitive
isspace is desirable even where libc provides isspace as a locale-sensitive
function, can we do this without all of that inlining?

[Mark Mentovai]

(Android doesn’t need to subscribe to my proposed default of using the standard
library.)

[digit]

I've rewritten the patch to suit your recommendations. Let me know what you
think about it.

[Mark Mentovai]

LGTM

http://breakpad.appspot.com/411002/diff/4001/src/common/linux/linux_libc_supp...
File src/common/linux/linux_libc_support.h (right):

http://breakpad.appspot.com/411002/diff/4001/src/common/linux/linux_libc_supp...
src/common/linux/linux_libc_support.h:142: static inline const char*
I’m still concerned about code size by having the remaining functions defined in
this .h rather than in some .cc. I know it’s the pattern that this file already
established so I’m not gonna keep you on the hook to fix it right now, but I
wonder if things might be better if we just stuck declarations here and put the
rest into a new linux_libc_support.cc. The problem’s only getting worse as we
add more things to this file.

[Ted Mielczarek]

https://breakpad.appspot.com/411002/diff/4001/src/client/linux/minidump_write...
File src/client/linux/minidump_writer/linux_dumper.cc (right):

https://breakpad.appspot.com/411002/diff/4001/src/client/linux/minidump_write...
src/client/linux/minidump_writer/linux_dumper.cc:158: close(fd);
sys_close here too.

[digit]

https://breakpad.appspot.com/411002/diff/4001/src/client/linux/minidump_write...
File src/client/linux/minidump_writer/linux_dumper.cc (right):

https://breakpad.appspot.com/411002/diff/4001/src/client/linux/minidump_write...
src/client/linux/minidump_writer/linux_dumper.cc:158: close(fd);
thanks, will fix.

https://breakpad.appspot.com/411002/diff/4001/src/common/linux/linux_libc_sup...
File src/common/linux/linux_libc_support.h (right):

https://breakpad.appspot.com/411002/diff/4001/src/common/linux/linux_libc_sup...
src/common/linux/linux_libc_support.h:142: static inline const char*
All right, I'll do this, no worry.

[digit]

Patch set 3 contains the fixes described above. The new source file is
src/ommon/linux/linux_libc_support.cc

[Mark Mentovai]

https://breakpad.appspot.com/411002/diff/10001/src/common/linux/linux_libc_su...
File src/common/linux/linux_libc_support.cc (right):

https://breakpad.appspot.com/411002/diff/10001/src/common/linux/linux_libc_su...
src/common/linux/linux_libc_support.cc:1: // Copyright (c) 2012, Google Inc.
If you’re using svn, can you create this file as an “svn copy” from the .h file?
It has pretty much the same content, and the “svn copy” would preserve the
revision history in this file. It’d also make it easier to see what’s changed
from the .h.

https://breakpad.appspot.com/411002/diff/10001/src/common/linux/linux_libc_su...
src/common/linux/linux_libc_support.cc:40: size_t
Our style doesn’t put the return type on its own line.

(I know the problem was present in the original .h file, but now’s a good time
to fix it.)

[digit]

https://breakpad.appspot.com/411002/diff/10001/src/common/linux/linux_libc_su...
File src/common/linux/linux_libc_support.cc (right):

https://breakpad.appspot.com/411002/diff/10001/src/common/linux/linux_libc_su...
src/common/linux/linux_libc_support.cc:1: // Copyright (c) 2012, Google Inc.
will do

https://breakpad.appspot.com/411002/diff/10001/src/common/linux/linux_libc_su...
src/common/linux/linux_libc_support.cc:40: size_t
will do

[Mark Mentovai]

LGTM