Explicitly CHECK arguments in dbus::MessageWriter::AppendString/ObjectPath

dbus/string_util.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 "dbus/string_util.h"
+
+#include "base/string_tokenizer.h"
+
+namespace dbus {
+
+bool IsStringValidObjectPath(const std::string& string) {
+  // This implementation is based upon D-Bus Specification Version 0.19.
+
+  // A valid object path begins with '/'.
+  if (string.empty())
+    return false;
+  if (string[0] != '/')
+    return false;

[satorux1, 2012/06/04 23:29:19]

maybe 

if (!StartsWithAscii(value, "/", true))
  return false;

[hashimoto, 2012/06/05 01:50:29]

Done.

+
+  int element_length = 0;

[satorux1, 2012/06/04 23:29:19]

might want to explain what element is.

// Elements are pieces delimited by '/'. For instance, "org", "chromium", "Foo"
are elements of "/org/chromium/Foo". 

[hashimoto, 2012/06/05 01:50:29]

Done.

+  for (size_t i = 1; i < string.length(); ++i) {

[satorux1, 2012/06/04 23:29:19]

nit: .length() -> .size() as the former is more common in chrome code

[satorux1, 2012/06/04 23:32:15]

I meant the latter (size()) is more common.

[hashimoto, 2012/06/05 01:50:29]

Done.

+    const char c = string[i];
+    if (c == '/') {
+      // No element separated by '/' may be the empty string.
+      if (element_length == 0)
+        return false;
+      element_length = 0;
+    } else {
+      // Each element must only contain "[A-Z][a-z][0-9]_".
+      const bool is_valid_character =
+          ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z') ||
+          ('0' <= c && c <= '9') || c == '_';
+      if (!is_valid_character)
+        return false;
+      element_length++;
+    }
+  }
+
+  // A trailing '/' character is not allowed unless the path is the root path.
+  if (string.length() > 1 && string[string.length() - 1] == '/')

[satorux1, 2012/06/04 23:29:19]

maybe

if (value.size() > 1 && EndsWithASCII(value, "/", true))

[hashimoto, 2012/06/05 01:50:29]

Done.

+    return false;
+
+  return true;
+}
+
+}  // namespace dbus