don't override libc code in a glibc-specific way when not using glibc

don't override libc code in a glibc-specific way when not using glibc

We should not attempt to override libc functions using glibc-specific
code when not actually using glibc.  This still needs to be fixed
for other libc's though.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=206040

[Mostyn Bramley-Moore, 2013-06-12 09:10:51]

This small patch adds an ifdef around some previously identified glibc-specific
code.

We're looking into a proper fix, but I think this makes sense to have in the
meantime.

[agl, 2013-06-12 19:19:56]

lgtm

I'm not OWNERS here, but I think this is fine with one nit.

https://codereview.chromium.org/16132015/diff/1/sandbox/linux/services/libc_u...
File sandbox/linux/services/libc_urandom_override.cc (right):

https://codereview.chromium.org/16132015/diff/1/sandbox/linux/services/libc_u...
sandbox/linux/services/libc_urandom_override.cc:62: #ifdef LIBC_GLIBC
#if defined(LIBC_GLIBC)

(seems to be the style in this file.)

[Mostyn Bramley-Moore, 2013-06-12 19:38:35]

https://codereview.chromium.org/16132015/diff/1/sandbox/linux/services/libc_u...
File sandbox/linux/services/libc_urandom_override.cc (right):

https://codereview.chromium.org/16132015/diff/1/sandbox/linux/services/libc_u...
sandbox/linux/services/libc_urandom_override.cc:62: #ifdef LIBC_GLIBC
On 2013/06/12 19:19:56, agl wrote:
> #if defined(LIBC_GLIBC)
> 
> (seems to be the style in this file.)

Done.

[jln (very slow on Chromium), 2013-06-12 20:27:19]

lgtm

[jln (very slow on Chromium), 2013-06-12 20:27:58]

https://codereview.chromium.org/16132015/diff/5001/sandbox/linux/services/lib...
File sandbox/linux/services/libc_urandom_override.cc (right):

https://codereview.chromium.org/16132015/diff/5001/sandbox/linux/services/lib...
sandbox/linux/services/libc_urandom_override.cc:159: #endif  // LIBC_GLIBC
Nit: this should be "// defined(LIBC_GLIBC)"

[Mostyn Bramley-Moore, 2013-06-12 20:41:41]

https://codereview.chromium.org/16132015/diff/5001/sandbox/linux/services/lib...
File sandbox/linux/services/libc_urandom_override.cc (right):

https://codereview.chromium.org/16132015/diff/5001/sandbox/linux/services/lib...
sandbox/linux/services/libc_urandom_override.cc:159: #endif  // LIBC_GLIBC
On 2013/06/12 20:27:59, Julien Tinnes wrote:
> Nit: this should be "// defined(LIBC_GLIBC)"

That doesn't match the format used two lines down, which has:
// !ADDRESS_SANITIZER

Would you prefer me to change both, or leave both?

[jln (very slow on Chromium), 2013-06-12 21:18:50]

On 2013/06/12 20:41:41, Mostyn Bramley-Moore wrote:
>
https://codereview.chromium.org/16132015/diff/5001/sandbox/linux/services/lib...
> File sandbox/linux/services/libc_urandom_override.cc (right):
> 
>
https://codereview.chromium.org/16132015/diff/5001/sandbox/linux/services/lib...
> sandbox/linux/services/libc_urandom_override.cc:159: #endif  // LIBC_GLIBC
> On 2013/06/12 20:27:59, Julien Tinnes wrote:
> > Nit: this should be "// defined(LIBC_GLIBC)"
> 
> That doesn't match the format used two lines down, which has:
> // !ADDRESS_SANITIZER
> 
> Would you prefer me to change both, or leave both?

If you can change both that's great, but don't sweat over it, you can also land
as-is.

[Mostyn Bramley-Moore, 2013-06-12 21:30:01]

> > Would you prefer me to change both, or leave both?
> 
> If you can change both that's great, but don't sweat over it, you can also
land
> as-is.

Done in patchset 3.

[commit-bot: I haz the power, 2013-06-12 21:30:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mostynb@opera.com/16132015/10001

[commit-bot: I haz the power, 2013-06-13 02:34:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mostynb@opera.com/16132015/10001

[commit-bot: I haz the power, 2013-06-13 09:19:47]

Change committed as 206040