A quick build fix for gcc 4.3 (and later)...

A build fix for gcc 4.3 (and later).
Comparing a function with NULL has been prohibited since gcc 4.3 and produces a compilation error.
This change encloses your code which compares a function with NULL with "#if defined(OS_MACOSX) && MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_5" and "#endif".

BUG=none
TEST=build chromium with gcc 4.3 (and later)

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=26449

[Mark Mentovai, 2009-09-16 06:22:45]

http://codereview.chromium.org/206024/diff/2001/3001
File base/debug_util_posix.cc (right):

http://codereview.chromium.org/206024/diff/2001/3001#newcode119
Line 119: #if defined(SUPPORT_MACOSX_10_4)
What is SUPPORT_MACOSX_10_4 supposed to control?

There is probably a better macro for your needs.

[Hironori Bono, 2009-09-16 07:30:53]

http://codereview.chromium.org/206024/diff/2001/3001
File base/debug_util_posix.cc (right):

http://codereview.chromium.org/206024/diff/2001/3001#newcode119
Line 119: #if defined(SUPPORT_MACOSX_10_4)
On 2009/09/16 06:22:45, Mark Mentovai wrote:
> What is SUPPORT_MACOSX_10_4 supposed to control?

This macro has been added by the original change that caused this build break:
<http://codereview.chromium.org/201122> "Changes needed for MacOS X 10.4
support". Even though I'm not sure the real usage of this macro, it seems to be
used for enclosing Tiger-specific code.

> There is probably a better macro for your needs.

I wish there is a better macro and he can write a better change than mine. :)

[Craig, 2009-09-17 05:57:08]

I have landed an alternative fix for this in r26431 (see 
http://codereview.chromium.org/206026/show) that squashes the warning with a
pragma. If anyone has strong feelings about this, feel free to revert it and
land something else or complain to me but at least the tree is happy for gcc >=
4.3.x for now :)

Thank you.

[Mark Mentovai, 2009-09-17 06:01:57]

LGTM with this change

http://codereview.chromium.org/206024/diff/6003/8001
File base/debug_util_posix.cc (right):

http://codereview.chromium.org/206024/diff/6003/8001#newcode123
Line 123: #if (defined(OS_MACOSX) && \
The outer layer of parentheses is not needed here or in any of the others you've
added.

[Mark Mentovai, 2009-09-17 06:03:31]

http://codereview.chromium.org/206024/diff/6003/8001
File base/debug_util_posix.cc (right):

http://codereview.chromium.org/206024/diff/6003/8001#newcode125
Line 125: if (backtrace == NULL) {
This can just be

|if (backtrace) {|

(throughout)

Technically, that's more correct than a direct equality comparison with NULL. 
Also, I prefer it without the |== NULL|.

[Mark Mentovai, 2009-09-17 06:11:16]

http://codereview.chromium.org/206024/diff/6003/8001
File base/debug_util_posix.cc (right):

http://codereview.chromium.org/206024/diff/6003/8001#newcode125
Line 125: if (backtrace == NULL) {
Obviously, I meant:

  if (!backtrace) {

[Craig, 2009-09-17 06:13:07]

Mark: I've reverted my change btw. - the tree broke for unrelated reasons and
reverting will hopefully make the tree green again.

Sorry.

[Hironori Bono, 2009-09-17 09:21:33]

mark,

Thank you for your reviews and comments.
I will land the updated change when the tree is back to open.

craig,

It is definitely not your fault but our fault because I also approved you to
land your change. So, you don't have to feel sorry.
Thank you so much for your contribution to the chromium project, and I wish you
continue writing great changes.

Regards,

Hironori Bono

[Craig, 2009-09-17 16:19:07]

On 2009/09/17 09:21:33, hbono wrote:
> I will land the updated change when the tree is back to open.

I just landed this for you ... unfortunately I'm not 100% clued up on landing
other people's patches so it went in without being attributed to you. My
apologies.

[maf, 2009-09-17 16:25:11]

http://codereview.chromium.org/206024/diff/6003/8001
File base/debug_util_posix.cc (right):

http://codereview.chromium.org/206024/diff/6003/8001#newcode125
Line 125: if (backtrace == NULL) {
No, you must explicitly compare to NULL or it won't work.  The compiler has a
special case for this and we have to hit it.

Maf 

On 2009/09/17 06:11:16, Mark Mentovai wrote:
> Obviously, I meant:
> 
>   if (!backtrace) {

[maf, 2009-09-17 16:28:18]

You must explicitly compare to NULL when checking for availability of
a weak imported function.  It's a special case.


Maf

On Wed, Sep 16, 2009 at 11:03 PM,  <mark@chromium.org> wrote:
>
> http://codereview.chromium.org/206024/diff/6003/8001
> File base/debug_util_posix.cc (right):
>
> http://codereview.chromium.org/206024/diff/6003/8001#newcode125
> Line 125: if (backtrace =3D=3D NULL) {
> This can just be
>
> |if (backtrace) {|
>
> (throughout)
>
> Technically, that's more correct than a direct equality comparison with
> NULL. =C2=A0Also, I prefer it without the |=3D=3D NULL|.
>
> http://codereview.chromium.org/206024
>

[Craig, 2009-09-17 16:42:04]

On 2009/09/17 16:28:18, maf wrote:
> You must explicitly compare to NULL when checking for availability of
> a weak imported function.  It's a special case.

Eeeek!

How about this?
http://codereview.chromium.org/214012/show

[Mark Mentovai, 2009-09-17 17:08:10]

maf@google.com wrote:
> http://codereview.chromium.org/206024/diff/6003/8001#newcode125
> Line 125: if (backtrace =3D=3D NULL) {
> No, you must explicitly compare to NULL or it won't work. =A0The compiler
> has a special case for this and we have to hit it.

What is this, CFM?  That's certainly not true for Mach-O.