Add array bounds assertion and improve type conversion for empty strings.

Add array bounds assertion and improve type conversion for empty strings.

BUG=324829
R=abarth@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238751

[abarth-chromium, 2013-12-04 18:00:30]

Lgtm

On Wednesday, December 4, 2013, wrote:

> Reviewers: abarth,
>
> Description:
> Add array bounds assertion and improve type conversion for empty strings.
>
> BUG=324829
>
> Please review this at https://codereview.chromium.org/105133002/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+27, -4 lines):
>   M mojo/public/bindings/lib/bindings.cc
>   M mojo/public/bindings/lib/bindings_internal.h
>   M mojo/public/tests/bindings_type_conversion_unittest.cc
>
>
> Index: mojo/public/bindings/lib/bindings.cc
> diff --git a/mojo/public/bindings/lib/bindings.cc
> b/mojo/public/bindings/lib/bindings.cc
> index 9f4d6fddc5f8921092cd78363de48c68eb08621a..
> a9d748f41c4b853507074e84bb1f9109016bd644 100644
> --- a/mojo/public/bindings/lib/bindings.cc
> +++ b/mojo/public/bindings/lib/bindings.cc
> @@ -10,21 +10,28 @@ namespace mojo {
>  String SimilarityTraits<String, std::string>::CopyFrom(const std::string&
> input,
>                                                         Buffer* buf) {
>    String::Builder result(input.size(), buf);
> -  memcpy(&result[0], input.data(), input.size());
> +  if (!input.empty())
> +    memcpy(&result[0], input.data(), input.size());
>    return result.Finish();
>  }
>  // static
>  std::string SimilarityTraits<String, std::string>::CopyTo(const String&
> input) {
> -  return input.is_null() ? std::string() :
> -                           std::string(&input[0], &input[0] +
> input.size());
> +  if (input.is_null() || input.size() == 0)
> +    return std::string();
> +
> +  return std::string(&input[0], &input[0] + input.size());
>  }
>
>  // static
>  String SimilarityTraits<String, const char*>::CopyFrom(const char* input,
>                                                         Buffer* buf) {
> +  if (!input)
> +    return String();
> +
>    size_t size = strlen(input);
>    String::Builder result(size, buf);
> -  memcpy(&result[0], input, size);
> +  if (size != 0)
> +    memcpy(&result[0], input, size);
>    return result.Finish();
>  }
>
> Index: mojo/public/bindings/lib/bindings_internal.h
> diff --git a/mojo/public/bindings/lib/bindings_internal.h
> b/mojo/public/bindings/lib/bindings_internal.h
> index 1d5a24a1e65352e6823aaa2bcf0776a8921bec25..
> c78db05830bd9a4820db46682cd84a92b3d4dc33 100644
> --- a/mojo/public/bindings/lib/bindings_internal.h
> +++ b/mojo/public/bindings/lib/bindings_internal.h
> @@ -163,10 +163,12 @@ class Array_Data {
>    size_t size() const { return header_.num_elements; }
>
>    T& at(size_t offset) {
> +    assert(offset < static_cast<size_t>(header_.num_elements));
>      return Traits::ToRef(storage()[offset]);
>    }
>
>    const T& at(size_t offset) const {
> +    assert(offset < static_cast<size_t>(header_.num_elements));
>      return Traits::ToConstRef(storage()[offset]);
>    }
>
> Index: mojo/public/tests/bindings_type_conversion_unittest.cc
> diff --git a/mojo/public/tests/bindings_type_conversion_unittest.cc
> b/mojo/public/tests/bindings_type_conversion_unittest.cc
> index 01050faf7075f7f789d4c87939062433fc6a698f..
> 6c41bb5c450adc158a68cabdd5667ca0d1c841dd 100644
> --- a/mojo/public/tests/bindings_type_conversion_unittest.cc
> +++ b/mojo/public/tests/bindings_type_conversion_unittest.cc
> @@ -85,7 +85,21 @@ TEST_F(BindingsTypeConversionTest, String) {
>
>  TEST_F(BindingsTypeConversionTest, String_Null) {
>    String a;
> +  EXPECT_TRUE(a.is_null());
>    EXPECT_EQ(std::string(), a.To<std::string>());
> +
> +  String b(static_cast<const char*>(NULL));
> +  EXPECT_TRUE(b.is_null());
> +}
> +
> +TEST_F(BindingsTypeConversionTest, String_Empty) {
> +  AllocationScope scope;
> +  String a = String::Builder(0).Finish();
> +  EXPECT_EQ(std::string(), a.To<std::string>());
> +
> +  String b = std::string();
> +  EXPECT_FALSE(b.is_null());
> +  EXPECT_EQ(std::string(), b.To<std::string>());
>  }
>
>  TEST_F(BindingsTypeConversionTest, String_ShallowCopy) {
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[darin (slow to review), 2013-12-04 20:27:49]

Committed patchset #2 manually as r238751 (presubmit successful).