Consistently suggest ephemeral port for QUIC client

net/quic/port_suggester.cc

+// Copyright (c) 2012 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/quic/port_suggester.h"
+
+#include "base/metrics/histogram.h"
+
+namespace net {
+
+PortSuggester::PortSuggester(const HostPortPair& hostport,
+                             uint64 per_profile_randomness)
+    : count_(0) {
+  base::SHA1HashBytes(
+      reinterpret_cast<const unsigned char*>(hostport.host().data()),
+      hostport.host().length(), seed_);
+
+  uint64* start_of_seed = reinterpret_cast<uint64*>(seed_);

[wtc, 2013/12/06 23:36:44]

This cast is not portable because not all processors allow unaligned access. The
cast on line 32 has the same problem.

A common way to do this properly is to use a union:

  union {
    unsigned char bytes[base::kSHA1Length];
    uint64 first_uint64;
    int first_int;
  } seed_;

[jar (doing other things), 2013/12/07 00:47:58]

Done.

+  DCHECK_LE(sizeof(*start_of_seed), sizeof(seed_));  // Safe memory access.
+  *start_of_seed ^= per_profile_randomness ^ hostport.port();
+}
+
+PortSuggester::~PortSuggester() {
+  UMA_HISTOGRAM_COUNTS("Net.Quic.EphemeralPortsSuggested", count_);
+}
+
+int PortSuggester::SuggestPort(int min, int max) {
+  // Sometimes our suggestion can't be used, so we ensure that if a 2nd call
+  // is made, that each call (probably) provides a new suggestion.

[wtc, 2013/12/06 23:36:44]

Nit: that each call => that call ?

[jar (doing other things), 2013/12/07 00:47:58]

Adjusted sentence.

+  if (++count_ > 1)
+    base::SHA1HashBytes(seed_, sizeof(seed_), seed_);  // Evolve the seed.
+  int rand = std::abs(*reinterpret_cast<int*>(seed_));
+  DCHECK_LE(sizeof(rand), sizeof(seed_));
+  DCHECK_LE(min, max);
+  DCHECK_GT(min, 0);
+  int range = max - min + 1;
+  return (rand % range) + min;

[wtc, 2013/12/06 23:36:44]

If |range| is not a power of 2, this results in a nonuniform distribution.
Please confirm that's not a problem.

[jar (doing other things), 2013/12/07 00:47:58]

Added comment.

+}
+
+}  // namespace net