Fix a ton of compiler warnings.

Most of these are classes with virtual methods lacking virtual destructors
or NULL used in non-pointer context.

BUG=none
TEST=app_unittests && base_unittests
--gtest_filter=-ConditionVariableTest.LargeFastTaskTest

patch by Jacob Mandelson <jlmjlm [at] gmail>
http://codereview.chromium.org/171028/show


git-svn-id: svn://svn.chromium.org/chrome/trunk/src@24792 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 55 insertions, 47 deletions

diff --git a/net/base/directory_lister.cc b/net/base/directory_lister.cc
index 616fc82..69e301e 100644
--- a/net/base/directory_lister.cc
+++ b/net/base/directory_lister.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -37,7 +37,7 @@ DirectoryLister::DirectoryLister(const FilePath& dir,
     : dir_(dir),
       delegate_(delegate),
       message_loop_(NULL),
-      thread_(NULL),
+      thread_(0),
       canceled_(false) {
   DCHECK(!dir.value().empty());
 }
@@ -70,7 +70,7 @@ void DirectoryLister::Cancel() {
 
   if (thread_) {
     PlatformThread::Join(thread_);
-    thread_ = NULL;
+    thread_ = 0;
   }
 }
 
diff --git a/net/base/directory_lister.h b/net/base/directory_lister.h
index cb98da6..58c2cd3 100644
--- a/net/base/directory_lister.h
+++ b/net/base/directory_lister.h
@@ -1,9 +1,9 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef NET_BASE_DIRECTORY_LISTER_H__
-#define NET_BASE_DIRECTORY_LISTER_H__
+#ifndef NET_BASE_DIRECTORY_LISTER_H_
+#define NET_BASE_DIRECTORY_LISTER_H_
 
 #include <string>
 
@@ -32,6 +32,9 @@ class DirectoryLister : public base::RefCountedThreadSafe<DirectoryLister>,
     virtual void OnListFile(
         const file_util::FileEnumerator::FindInfo& data) = 0;
     virtual void OnListDone(int error) = 0;
+
+   protected:
+    ~DirectoryListerDelegate() {}
   };
 
   DirectoryLister(const FilePath& dir, DirectoryListerDelegate* delegate);
@@ -68,4 +71,4 @@ class DirectoryLister : public base::RefCountedThreadSafe<DirectoryLister>,
 
 }  // namespace net
 
-#endif  // NET_BASE_DIRECTORY_LISTER_H__
+#endif  // NET_BASE_DIRECTORY_LISTER_H_
diff --git a/net/base/host_cache_unittest.cc b/net/base/host_cache_unittest.cc
index fe01e20..bfb1860 100644
--- a/net/base/host_cache_unittest.cc
+++ b/net/base/host_cache_unittest.cc
@@ -28,7 +28,7 @@ TEST(HostCacheTest, Basic) {
   EXPECT_EQ(0U, cache.size());
 
   // Add an entry for "foobar.com" at t=0.
-  EXPECT_EQ(NULL, cache.Lookup("foobar.com", base::TimeTicks()));
+  EXPECT_TRUE(NULL == cache.Lookup("foobar.com", base::TimeTicks()));
   cache.Set("foobar.com", OK, AddressList(), now);
   entry1 = cache.Lookup("foobar.com", base::TimeTicks());
   EXPECT_FALSE(NULL == entry1);
@@ -38,7 +38,7 @@ TEST(HostCacheTest, Basic) {
   now += base::TimeDelta::FromSeconds(5);
 
   // Add an entry for "foobar2.com" at t=5.
-  EXPECT_EQ(NULL, cache.Lookup("foobar2.com", base::TimeTicks()));
+  EXPECT_TRUE(NULL == cache.Lookup("foobar2.com", base::TimeTicks()));
   cache.Set("foobar2.com", OK, AddressList(), now);
   entry2 = cache.Lookup("foobar2.com", base::TimeTicks());
   EXPECT_FALSE(NULL == entry1);
@@ -54,7 +54,7 @@ TEST(HostCacheTest, Basic) {
   // Advance to t=10; entry1 is now expired.
   now += base::TimeDelta::FromSeconds(1);
 
-  EXPECT_EQ(NULL, cache.Lookup("foobar.com", now));
+  EXPECT_TRUE(NULL == cache.Lookup("foobar.com", now));
   EXPECT_EQ(entry2, cache.Lookup("foobar2.com", now));
 
   // Update entry1, so it is no longer expired.
@@ -70,8 +70,8 @@ TEST(HostCacheTest, Basic) {
   // Advance to t=20; both entries are now expired.
   now += base::TimeDelta::FromSeconds(10);
 
-  EXPECT_EQ(NULL, cache.Lookup("foobar.com", now));
-  EXPECT_EQ(NULL, cache.Lookup("foobar2.com", now));
+  EXPECT_TRUE(NULL == cache.Lookup("foobar.com", now));
+  EXPECT_TRUE(NULL == cache.Lookup("foobar2.com", now));
 }
 
 // Try caching entries for a failed resolve attempt.
@@ -81,19 +81,19 @@ TEST(HostCacheTest, NegativeEntry) {
   // Set t=0.
   base::TimeTicks now;
 
-  EXPECT_EQ(NULL, cache.Lookup("foobar.com", base::TimeTicks()));
+  EXPECT_TRUE(NULL == cache.Lookup("foobar.com", base::TimeTicks()));
   cache.Set("foobar.com", ERR_NAME_NOT_RESOLVED, AddressList(), now);
   EXPECT_EQ(1U, cache.size());
 
   // We disallow use of negative entries.
-  EXPECT_EQ(NULL, cache.Lookup("foobar.com", now));
+  EXPECT_TRUE(NULL == cache.Lookup("foobar.com", now));
 
   // Now overwrite with a valid entry, and then overwrite with negative entry
   // again -- the valid entry should be kicked out.
   cache.Set("foobar.com", OK, AddressList(), now);
   EXPECT_FALSE(NULL == cache.Lookup("foobar.com", now));
   cache.Set("foobar.com", ERR_NAME_NOT_RESOLVED, AddressList(), now);
-  EXPECT_EQ(NULL, cache.Lookup("foobar.com", now));
+  EXPECT_TRUE(NULL == cache.Lookup("foobar.com", now));
 }
 
 TEST(HostCacheTest, Compact) {
@@ -185,7 +185,7 @@ TEST(HostCacheTest, SetWithCompact) {
   // Adding the fourth entry will cause "expired" to be evicted.
   cache.Set("host3", OK, AddressList(), now);
   EXPECT_EQ(3U, cache.size());
-  EXPECT_EQ(NULL, cache.Lookup("expired", now));
+  EXPECT_TRUE(NULL == cache.Lookup("expired", now));
   EXPECT_FALSE(NULL == cache.Lookup("host1", now));
   EXPECT_FALSE(NULL == cache.Lookup("host2", now));
   EXPECT_FALSE(NULL == cache.Lookup("host3", now));
@@ -208,9 +208,9 @@ TEST(HostCacheTest, NoCache) {
   base::TimeTicks now;
 
   // Lookup and Set should have no effect.
-  EXPECT_EQ(NULL, cache.Lookup("foobar.com", base::TimeTicks()));
+  EXPECT_TRUE(NULL == cache.Lookup("foobar.com", base::TimeTicks()));
   cache.Set("foobar.com", OK, AddressList(), now);
-  EXPECT_EQ(NULL, cache.Lookup("foobar.com", base::TimeTicks()));
+  EXPECT_TRUE(NULL == cache.Lookup("foobar.com", base::TimeTicks()));
 
   EXPECT_EQ(0U, cache.size());
 }
diff --git a/net/base/ssl_client_auth_cache_unittest.cc b/net/base/ssl_client_auth_cache_unittest.cc
index 33eb25f..c0697fc 100644
--- a/net/base/ssl_client_auth_cache_unittest.cc
+++ b/net/base/ssl_client_auth_cache_unittest.cc
@@ -28,7 +28,7 @@ TEST(SSLClientAuthCacheTest, LookupAddRemove) {
     new X509Certificate("foo3", "CA", start_date, expiration_date));
 
   // Lookup non-existent client certificate.
-  EXPECT_EQ(NULL, cache.Lookup(server1));
+  EXPECT_TRUE(NULL == cache.Lookup(server1));
 
   // Add client certificate for server1.
   cache.Add(server1, cert1.get());
@@ -46,12 +46,12 @@ TEST(SSLClientAuthCacheTest, LookupAddRemove) {
 
   // Remove client certificate of server1.
   cache.Remove(server1);
-  EXPECT_EQ(NULL, cache.Lookup(server1));
+  EXPECT_TRUE(NULL == cache.Lookup(server1));
   EXPECT_EQ(cert2.get(), cache.Lookup(server2));
 
   // Remove non-existent client certificate.
   cache.Remove(server1);
-  EXPECT_EQ(NULL, cache.Lookup(server1));
+  EXPECT_TRUE(NULL == cache.Lookup(server1));
   EXPECT_EQ(cert2.get(), cache.Lookup(server2));
 }
 
diff --git a/net/ftp/ftp_auth_cache_unittest.cc b/net/ftp/ftp_auth_cache_unittest.cc
index f74762b..a4d8ef4 100644
--- a/net/ftp/ftp_auth_cache_unittest.cc
+++ b/net/ftp/ftp_auth_cache_unittest.cc
@@ -24,7 +24,7 @@ TEST(FtpAuthCacheTest, LookupAddRemove) {
   scoped_refptr<AuthData> data3(new AuthData());
 
   // Lookup non-existent entry.
-  EXPECT_EQ(NULL, cache.Lookup(origin1));
+  EXPECT_TRUE(NULL == cache.Lookup(origin1));
 
   // Add entry for origin1.
   cache.Add(origin1, data1.get());
@@ -42,12 +42,12 @@ TEST(FtpAuthCacheTest, LookupAddRemove) {
 
   // Remove entry of origin1.
   cache.Remove(origin1);
-  EXPECT_EQ(NULL, cache.Lookup(origin1));
+  EXPECT_TRUE(NULL == cache.Lookup(origin1));
   EXPECT_EQ(data2.get(), cache.Lookup(origin2));
 
   // Remove non-existent entry
   cache.Remove(origin1);
-  EXPECT_EQ(NULL, cache.Lookup(origin1));
+  EXPECT_TRUE(NULL == cache.Lookup(origin1));
   EXPECT_EQ(data2.get(), cache.Lookup(origin2));
 }
 
@@ -93,7 +93,7 @@ TEST(FtpAuthCacheTest, NormalizedKey) {
 
   // Remove
   cache.Remove(GURL("ftp://HOsT"));
-  EXPECT_EQ(NULL, cache.Lookup(GURL("ftp://host")));
+  EXPECT_TRUE(NULL == cache.Lookup(GURL("ftp://host")));
 }
 
 TEST(FtpAuthCacheTest, EvictOldEntries) {
@@ -112,7 +112,7 @@ TEST(FtpAuthCacheTest, EvictOldEntries) {
 
   // Adding one entry should cause eviction of the first entry.
   cache.Add(GURL("ftp://last_host"), auth_data.get());
-  EXPECT_EQ(NULL, cache.Lookup(GURL("ftp://host0")));
+  EXPECT_TRUE(NULL == cache.Lookup(GURL("ftp://host0")));
 
   // Remaining entries should not get evicted.
   for (size_t i = 1; i < FtpAuthCache::kMaxEntries; i++) {
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index d63546d..b4d99d6 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -2639,19 +2639,19 @@ TEST_F(HttpNetworkTransactionTest, ResetStateForRestart) {
   trans->ResetStateForRestart();
 
   // Verify that the state that needed to be reset, has been reset.
-  EXPECT_EQ(NULL, trans->header_buf_->headers());
+  EXPECT_TRUE(NULL == trans->header_buf_->headers());
   EXPECT_EQ(0, trans->header_buf_capacity_);
   EXPECT_EQ(0, trans->header_buf_len_);
   EXPECT_EQ(-1, trans->header_buf_body_offset_);
   EXPECT_EQ(-1, trans->header_buf_http_offset_);
   EXPECT_EQ(-1, trans->response_body_length_);
   EXPECT_EQ(0, trans->response_body_read_);
-  EXPECT_EQ(NULL, trans->read_buf_.get());
+  EXPECT_TRUE(NULL == trans->read_buf_.get());
   EXPECT_EQ(0, trans->read_buf_len_);
   EXPECT_EQ("", trans->request_headers_->headers_);
   EXPECT_EQ(0U, trans->request_headers_bytes_sent_);
-  EXPECT_EQ(NULL, trans->response_.auth_challenge.get());
-  EXPECT_EQ(NULL, trans->response_.headers.get());
+  EXPECT_TRUE(NULL == trans->response_.auth_challenge.get());
+  EXPECT_TRUE(NULL == trans->response_.headers.get());
   EXPECT_EQ(false, trans->response_.was_cached);
   EXPECT_EQ(0, trans->response_.ssl_info.cert_status);
   EXPECT_FALSE(trans->response_.vary_data.is_valid());
diff --git a/net/proxy/single_threaded_proxy_resolver_unittest.cc b/net/proxy/single_threaded_proxy_resolver_unittest.cc
index da21ff1..0503a01 100644
--- a/net/proxy/single_threaded_proxy_resolver_unittest.cc
+++ b/net/proxy/single_threaded_proxy_resolver_unittest.cc
@@ -34,8 +34,8 @@ class MockProxyResolver : public ProxyResolver {
 
     CheckIsOnWorkerThread();
 
-    EXPECT_EQ(NULL, callback);
-    EXPECT_EQ(NULL, request);
+    EXPECT_TRUE(NULL == callback);
+    EXPECT_TRUE(NULL == request);
 
     results->UseNamedProxy(query_url.host());
 
diff --git a/net/socket/client_socket_pool_base.h b/net/socket/client_socket_pool_base.h
index e6368ca..9beac0a 100644
--- a/net/socket/client_socket_pool_base.h
+++ b/net/socket/client_socket_pool_base.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
@@ -426,7 +426,7 @@ class ClientSocketPoolBase {
           max_sockets, max_sockets_per_group,
           new ConnectJobFactoryAdaptor(connect_job_factory))) {}
 
-  ~ClientSocketPoolBase() {}
+  virtual ~ClientSocketPoolBase() {}
 
   // These member functions simply forward to ClientSocketPoolBaseHelper.
 
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index fff6869..5f33197 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -327,9 +327,9 @@ class TestConnectJobDelegate : public ConnectJob::Delegate {
     result_ = result;
     scoped_ptr<ClientSocket> socket(job->ReleaseSocket());
     if (result == OK) {
-      EXPECT_TRUE(socket.get() != NULL);
+      EXPECT_FALSE(socket.get() == NULL);
     } else {
-      EXPECT_EQ(NULL, socket.get());
+      EXPECT_TRUE(socket.get() == NULL);
     }
     delete job;
     have_result_ = true;
diff --git a/net/socket/ssl_test_util.cc b/net/socket/ssl_test_util.cc
index 7e73f1e..e60cab9 100644
--- a/net/socket/ssl_test_util.cc
+++ b/net/socket/ssl_test_util.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -136,7 +136,7 @@ const int TestServerLauncher::kBadHTTPSPort = 9666;
 // The issuer name of the cert that should be trusted for the test to work.
 const wchar_t TestServerLauncher::kCertIssuerName[] = L"Test CA";
 
-TestServerLauncher::TestServerLauncher() : process_handle_(NULL),
+TestServerLauncher::TestServerLauncher() : process_handle_(0),
                                            forking_(false),
                                            connection_attempts_(10),
                                            connection_timeout_(1000)
@@ -149,7 +149,7 @@ TestServerLauncher::TestServerLauncher() : process_handle_(NULL),
 
 TestServerLauncher::TestServerLauncher(int connection_attempts,
                                        int connection_timeout)
-                        : process_handle_(NULL),
+                        : process_handle_(0),
                           forking_(false),
                           connection_attempts_(connection_attempts),
                           connection_timeout_(connection_timeout)
@@ -331,7 +331,7 @@ bool TestServerLauncher::WaitToFinish(int timeout_ms) {
   bool ret = base::WaitForSingleProcess(process_handle_, timeout_ms);
   if (ret) {
     base::CloseProcessHandle(process_handle_);
-    process_handle_ = NULL;
+    process_handle_ = 0;
     LOG(INFO) << "Finished.";
   } else {
     LOG(INFO) << "Timed out.";
@@ -346,7 +346,7 @@ bool TestServerLauncher::Stop() {
   bool ret = base::KillProcess(process_handle_, 1, true);
   if (ret) {
     base::CloseProcessHandle(process_handle_);
-    process_handle_ = NULL;
+    process_handle_ = 0;
     LOG(INFO) << "Stopped.";
   } else {
     LOG(INFO) << "Kill failed?";
diff --git a/net/tools/fetch/http_listen_socket.h b/net/tools/fetch/http_listen_socket.h
index f31df20..81f0de0 100644
--- a/net/tools/fetch/http_listen_socket.h
+++ b/net/tools/fetch/http_listen_socket.h
@@ -17,8 +17,11 @@ class HttpListenSocket : public ListenSocket,
 public:
   class Delegate {
    public:
-    virtual void OnRequest(HttpListenSocket* connection, 
+    virtual void OnRequest(HttpListenSocket* connection,
                            HttpServerRequestInfo* info) = 0;
+
+   protected:
+    ~Delegate() {}
   };
 
   static HttpListenSocket* Listen(const std::string& ip, int port,
diff --git a/net/url_request/url_request_unittest.h b/net/url_request/url_request_unittest.h
index e058957..3c51571 100644
--- a/net/url_request/url_request_unittest.h
+++ b/net/url_request/url_request_unittest.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -239,11 +239,13 @@ class TestDelegate : public URLRequest::Delegate {
 // that can provide various responses useful for testing.
 class BaseTestServer : public base::RefCounted<BaseTestServer> {
  protected:
-  BaseTestServer() { }
+  BaseTestServer() {}
   BaseTestServer(int connection_attempts, int connection_timeout)
-      : launcher_(connection_attempts, connection_timeout) { }
+      : launcher_(connection_attempts, connection_timeout) {}
 
  public:
+  virtual ~BaseTestServer() {}
+
   void set_forking(bool forking) {
     launcher_.set_forking(forking);
   }