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

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

Add dbus::IsStringValidObjectPath() and dbus::ObjectPath::IsValid()

BUG=129335
TEST=dbus_unittests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=140489

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

LGTM with nits:

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc
File dbus/string_util.cc (right):

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc#newcode18
dbus/string_util.cc:18: return false;
maybe 

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

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc#newcode20
dbus/string_util.cc:20: int element_length = 0;
might want to explain what element is.

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

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc#newcode21
dbus/string_util.cc:21: for (size_t i = 1; i < string.length(); ++i) {
nit: .length() -> .size() as the former is more common in chrome code

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc#newcode40
dbus/string_util.cc:40: if (string.length() > 1 && string[string.length() - 1]
== '/')
maybe

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

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.h
File dbus/string_util.h (right):

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.h#newcode14
dbus/string_util.h:14: bool IsStringValidObjectPath(const std::string& string);
IsValidObjectPath() should suffice.

string -> value?

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

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc
File dbus/string_util.cc (right):

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc#newcode21
dbus/string_util.cc:21: for (size_t i = 1; i < string.length(); ++i) {
On 2012/06/04 23:29:19, satorux1 wrote:
> nit: .length() -> .size() as the former is more common in chrome code

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

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

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc
File dbus/string_util.cc (right):

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc#newcode18
dbus/string_util.cc:18: return false;
On 2012/06/04 23:29:19, satorux1 wrote:
> maybe 
> 
> if (!StartsWithAscii(value, "/", true))
>   return false;

Done.

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc#newcode20
dbus/string_util.cc:20: int element_length = 0;
On 2012/06/04 23:29:19, satorux1 wrote:
> might want to explain what element is.
> 
> // Elements are pieces delimited by '/'. For instance, "org", "chromium",
"Foo"
> are elements of "/org/chromium/Foo". 

Done.

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc#newcode21
dbus/string_util.cc:21: for (size_t i = 1; i < string.length(); ++i) {
On 2012/06/04 23:29:19, satorux1 wrote:
> nit: .length() -> .size() as the former is more common in chrome code

Done.

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.cc#newcode40
dbus/string_util.cc:40: if (string.length() > 1 && string[string.length() - 1]
== '/')
On 2012/06/04 23:29:19, satorux1 wrote:
> maybe
> 
> if (value.size() > 1 && EndsWithASCII(value, "/", true))

Done.

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.h
File dbus/string_util.h (right):

http://codereview.chromium.org/10502011/diff/9001/dbus/string_util.h#newcode14
dbus/string_util.h:14: bool IsStringValidObjectPath(const std::string& string);
On 2012/06/04 23:29:19, satorux1 wrote:
> IsValidObjectPath() should suffice.
> 
> string -> value?

Done.

[commit-bot: I haz the power, 2012-06-05 03:11:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hashimoto@chromium.org/10502011/12001

[commit-bot: I haz the power, 2012-06-05 04:15:52]

Change committed as 140489