Explicitly compare to NULL when looking for weak_import symbols.

Explicitly compare to NULL when looking for weak_import symbols.

Changing the comparison form affects the warning messages
generated in various circumstances. See the codereview for
details.

Also, Apple docs claim that !symbol comparisons will not work.

Spotted by Matthew Vosburgh.

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

[Craig, 2009-09-17 17:08:02]

Please review.

Original issue at http://codereview.chromium.org/206024/show

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

! is fine.

[Craig, 2009-09-17 17:10:26]

On 2009/09/17 17:08:02, Craig wrote:
> Please review.
> 
> Original issue at http://codereview.chromium.org/206024/show

And there is confirmation of the compiler special casing NULL here:
http://developer.apple.com/mac/library/documentation/MacOSX/Conceptual/BPFram...

[maf, 2009-09-17 20:30:44]

LGTM

[Craig, 2009-09-18 05:08:38]

On 2009/09/17 20:30:44, maf wrote:
> LGTM

Do you think this patch is going to make a difference given Mark's comments? I
unfortunately don't have a Intel Mac to try this on.

Thank you.

[maf, 2009-09-18 05:17:23]

I disagree with Mark.

On some configurations the symbol is weakly defined and present, on
some it's weakly defined and absent, and on some it is strongly
defined and always present.

If the symbol is strongly defined this causes a compiler warning from
GCC (saying the expression is always true):
  if (myFunction)
whereas this does not:
  if (myFunction == NULL)

So explicitly comparing with NULL has an advantage beyond the compiler
support which may or may not still be required.
I was following this documentation which was mentioned earlier in the thread.
http://developer.apple.com/mac/library/documentation/MacOSX/Conceptual/BPFram...


Maf
Maf

On Thu, Sep 17, 2009 at 10:08 PM,  <craig.schlenter@chromium.org> wrote:
> On 2009/09/17 20:30:44, maf wrote:
>>
>> LGTM
>
> Do you think this patch is going to make a difference given Mark's comments?
> I
> unfortunately don't have a Intel Mac to try this on.
>
> Thank you.
>
> http://codereview.chromium.org/214012
>