Convert plain C-style casts to use CFCastStrict and GetValueFromDictionary template

net/proxy/proxy_config_service_mac.cc

 // Copyright (c) 2011 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.
 
 #include "net/proxy/proxy_config_service_mac.h"
 
 #include <CoreFoundation/CoreFoundation.h>
 #include <SystemConfiguration/SystemConfiguration.h>
 
 #include "base/logging.h"
+#include "base/mac/foundation_util.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/message_loop.h"
 #include "base/sys_string_conversions.h"
 #include "net/base/net_errors.h"
 #include "net/proxy/proxy_config.h"
 #include "net/proxy/proxy_info.h"
 #include "net/proxy/proxy_server.h"
 
 namespace net {
 
 namespace {
 
 const int kPollIntervalSec = 5;
 
 // Utility function to pull out a boolean value from a dictionary and return it,
 // returning a default value if the key is not present.
 bool GetBoolFromDictionary(CFDictionaryRef dict,
                            CFStringRef key,
                            bool default_value) {
-  CFNumberRef number = (CFNumberRef)base::mac::GetValueFromDictionary(
-      dict, key, CFNumberGetTypeID());
+  CFNumberRef number =
+      base::mac::CFCastStrict<CFNumberRef>(

[Ryan Sleevi, 2011/11/11 00:26:42]

nit: You can align this to the previous line, and then you only have to indent
line 34 by four spaces, rather than 8.

This also applies to the following lines: 65-70, 137-140

However, I don't believe any of these casts are needed to be strict (except for
144/145), as GetValueFromDictionary already asserts that the returned type is
the same as the expected type.

If they aren't, it will DLOG(ERROR) and return NULL, which accomplishes the same
as the Strict check.

So perhaps just CFCast these?

[KushalP, 2011/11/11 11:03:19]

In the case of GetValueFromDictionary() it should definitely use the Strict case
as we've already partially told it what to expect in the bucket.

I'll update the CFArrayGetValueAtIndex cases to use the non-strict variant.

On 2011/11/11 00:26:42, Ryan Sleevi wrote:

[Ryan Sleevi, 2011/11/11 17:23:11]

I think you misunderstood. I believe CFArrayGetValueAtIndex *should* be using
the strict.

However, please examine the implementation of GetValueFromDictionary. You'll see
that it's already doing strict checks - if the value it extracts from the
dictionary is not of the same CFType requested, it will DLOG(WARNING) and return
NULL.

+          base::mac::GetValueFromDictionary(dict, key, CFNumberGetTypeID()));
   if (!number)
     return default_value;
 
   int int_value;
   if (CFNumberGetValue(number, kCFNumberIntType, &int_value))
     return int_value;
   else
     return default_value;
 }
 
@@ skipping 10 matching lines @@
   config->set_auto_detect(
       GetBoolFromDictionary(config_dict.get(),
                             kSCPropNetProxiesProxyAutoDiscoveryEnable,
                             false));
 
   // PAC file
 
   if (GetBoolFromDictionary(config_dict.get(),
                             kSCPropNetProxiesProxyAutoConfigEnable,
                             false)) {
-    CFStringRef pac_url_ref = (CFStringRef)base::mac::GetValueFromDictionary(
-        config_dict.get(),
-        kSCPropNetProxiesProxyAutoConfigURLString,
-        CFStringGetTypeID());
+    CFStringRef pac_url_ref =
+        base::mac::CFCastStrict<CFStringRef>(
+            base::mac::GetValueFromDictionary(
+                config_dict.get(),
+                kSCPropNetProxiesProxyAutoConfigURLString,
+                CFStringGetTypeID()));
     if (pac_url_ref)
       config->set_pac_url(GURL(base::SysCFStringRefToUTF8(pac_url_ref)));
   }
 
   // proxies (for now ftp, http, https, and SOCKS)
 
   if (GetBoolFromDictionary(config_dict.get(),
                             kSCPropNetProxiesFTPEnable,
                             false)) {
     ProxyServer proxy_server =
@@ skipping 46 matching lines @@
     if (proxy_server.is_valid()) {
       config->proxy_rules().type =
           ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME;
       config->proxy_rules().fallback_proxy = proxy_server;
     }
   }
 
   // proxy bypass list
 
   CFArrayRef bypass_array_ref =
-      (CFArrayRef)base::mac::GetValueFromDictionary(
+      base::mac::CFCastStrict<CFArrayRef>(base::mac::GetValueFromDictionary(
           config_dict.get(),
           kSCPropNetProxiesExceptionsList,
-          CFArrayGetTypeID());
+          CFArrayGetTypeID()));
   if (bypass_array_ref) {
     CFIndex bypass_array_count = CFArrayGetCount(bypass_array_ref);
     for (CFIndex i = 0; i < bypass_array_count; ++i) {
-      CFStringRef bypass_item_ref =
-          (CFStringRef)CFArrayGetValueAtIndex(bypass_array_ref, i);
+      CFStringRef bypass_item_ref = base::mac::CFCastStrict<CFStringRef>(
+              CFArrayGetValueAtIndex(bypass_array_ref, i));
       if (CFGetTypeID(bypass_item_ref) != CFStringGetTypeID()) {
         LOG(WARNING) << "Expected value for item " << i
                      << " in the kSCPropNetProxiesExceptionsList"
                         " to be a CFStringRef but it was not";
 
       } else {
         config->proxy_rules().bypass_rules.AddRuleFromString(
             base::SysCFStringRefToUTF8(bypass_item_ref));
       }
     }
@@ skipping 112 matching lines @@
   // Keep track of the last value we have seen.
   has_fetched_config_ = true;
   last_config_fetched_ = new_config;
 
   // Notify all the observers.
   FOR_EACH_OBSERVER(Observer, observers_,
                     OnProxyConfigChanged(new_config, CONFIG_VALID));
 }
 
 }  // namespace net