Safe-browsing PrefixSet cleanups.

chrome/browser/safe_browsing/prefix_set.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 "chrome/browser/safe_browsing/prefix_set.h"
 
 #include <algorithm>
 #include <math.h>
 
 #include "base/file_util.h"
@@ skipping 36 matching lines @@
     index_.push_back(std::make_pair(prev_prefix, deltas_.size()));
 
     for (size_t i = 1; i < sorted_prefixes.size(); ++i) {
       // Skip duplicates.
       if (sorted_prefixes[i] == prev_prefix)
         continue;
 
       // Calculate the delta.  |unsigned| is mandatory, because the
       // sorted_prefixes could be more than INT_MAX apart.
       DCHECK_GT(sorted_prefixes[i], prev_prefix);
-      unsigned delta = sorted_prefixes[i] - prev_prefix;
+      const unsigned delta = sorted_prefixes[i] - prev_prefix;
+      const uint16 delta16 = static_cast<uint16>(delta);
 
-      // New index ref if the delta is too wide, or if too many
-      // consecutive deltas have been encoded.  Note that
-      // |kMaxDelta| cannot itself be encoded.
-      if (delta >= kMaxDelta || run_length >= kMaxRun) {
+      // New index ref if the delta doesn't fit, or if too many
+      // consecutive deltas have been encoded.
+      if (delta != static_cast<unsigned>(delta16) || run_length >= kMaxRun) {

[lzheng, 2011/03/18 00:37:34]

I feel delta >= kMaxDelta is the same as delta !=
static_cast<unsigned>(delta16). Can you confirm?

[Scott Hess - ex-Googler, 2011/03/18 02:04:59]

Should be identical.  It was one of the notions that occurred to me while going
over things looking for possible off-by-one and other edge cases.

It also occurred to me that we never encode 0, so we could use that, but I
decided that the gain was really minimal for the complexity (the "delta too
large" case only fires a dozen times or so for the current safe-browsing list,
so 65536 might never be encoded ever).

         index_.push_back(std::make_pair(sorted_prefixes[i], deltas_.size()));
         run_length = 0;
       } else {
         // Continue the run of deltas.
-        deltas_.push_back(static_cast<uint16>(delta));
+        deltas_.push_back(delta16);
+        DCHECK_EQ(static_cast<unsigned>(deltas_.back()), delta);
         ++run_length;
       }
 
       prev_prefix = sorted_prefixes[i];
     }
 
     // Send up some memory-usage stats.  Bits because fractional bytes
     // are weird.
     const size_t bits_used = index_.size() * sizeof(index_[0]) * CHAR_BIT +
         deltas_.size() * sizeof(deltas_[0]) * CHAR_BIT;
@@ skipping 40 matching lines @@
     return true;
 
   // Scan forward accumulating deltas while a match is possible.
   for (size_t di = iter->second; di < bound && current < prefix; ++di) {
     current += deltas_[di];
   }
 
   return current == prefix;
 }
 
-void PrefixSet::GetPrefixes(std::vector<SBPrefix>* prefixes) {
+void PrefixSet::GetPrefixes(std::vector<SBPrefix>* prefixes) const {
   for (size_t ii = 0; ii < index_.size(); ++ii) {
     // The deltas for this |index_| entry run to the next index entry,
     // or the end of the deltas.
     const size_t deltas_end =
         (ii + 1 < index_.size()) ? index_[ii + 1].second : deltas_.size();
 
     SBPrefix current = index_[ii].first;
     prefixes->push_back(current);
     for (size_t di = index_[ii].second; di < deltas_end; ++di) {
       current += deltas_[di];
@@ skipping 119 matching lines @@
   if (written != 1)
     return false;
 
   // TODO(shess): Can this code check that the close was successful?
   file.reset();
 
   return true;
 }
 
 }  // namespace safe_browsing