Closed Bug 1073711 Opened 5 years ago Closed 5 years ago

Clang 3.5 build warning (promoted w/ werror): "netwerk/cache/nsCacheService.cpp:1326:9: error: 'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false [-Werror,-Wtautological-undefined-compare]"

Categories

(Core :: Networking, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: dholbert, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I get the following build warning with clang 3.5 in Ubuntu 14.10 final beta:
{
netwerk/cache/nsCacheService.cpp:1326:9: error: 'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false [-Werror,-Wtautological-undefined-compare]
     if (this == nullptr)  return NS_ERROR_NOT_AVAILABLE;
         ^~~~    ~~~~~~~
}

(I think this is a trunk-ish clang version, so other "clang 3.5" releases might not have the same warning; this explains why no one has hit this yet.)


This line of code dates back to 2001, it looks like:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/netwerk/cache/src/nsCacheService.cpp&rev=1.120#707

Filing this bug on fixing this.
Attached patch fix v1Splinter Review
This patch:
 (1) drops the null-check of 'this'
 (2) drops null-check after 'new'
 (3) Uses nsRefPtr for cleaner/more foolproof management of the returned object -- in particular:
 (3a) the local nsRefPtr<> does the AddRef implicitly (so no need for explicit NS_ADDREF)
 (3b) nsRefPtr::forget() does the assignment to the outparam for us. (instead of needing explicit raw-pointer dereferencing/assignment)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #8496295 - Flags: review?(jduell.mcbugs)
thanks daniel - I just want to make sure honza sees all the cache changes.
Attachment #8496295 - Flags: review?(jduell.mcbugs) → review?(honzab.moz)
Attachment #8496295 - Flags: review?(honzab.moz) → review+
https://hg.mozilla.org/mozilla-central/rev/3bea361c1f40
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.