From 0f95fa4909ba6f65364cb0b5c8c37a27c09b67b4 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Tue, 18 Jul 2017 10:42:33 -0700 Subject: [PATCH] Add idiomatic C++ API for grpc::Slice construction that doesn't require using grpc_slice --- include/grpc++/support/slice.h | 14 ++++++++++ src/cpp/util/slice_cc.cc | 13 +++++++++ test/cpp/qps/client.h | 4 +-- test/cpp/util/byte_buffer_proto_helper.cc | 3 +-- test/cpp/util/byte_buffer_test.cc | 21 +++++++-------- test/cpp/util/cli_call.cc | 3 +-- test/cpp/util/slice_test.cc | 33 +++++++++++++++++++---- 7 files changed, 67 insertions(+), 24 deletions(-) diff --git a/include/grpc++/support/slice.h b/include/grpc++/support/slice.h index db5cf942a0c..0b4ba7cecb4 100644 --- a/include/grpc++/support/slice.h +++ b/include/grpc++/support/slice.h @@ -44,6 +44,20 @@ class Slice final { /// Construct a slice from \a slice, stealing a reference. Slice(grpc_slice slice, StealRef); + /// Allocate a slice of specified size + Slice(size_t len); + + /// Construct a slice from a copied buffer + Slice(const void* buf, size_t len); + + /// Construct a slice from a copied string + Slice(const grpc::string& str); + + enum StaticSlice { STATIC_SLICE }; + + /// Construct a slice from a static buffer + Slice(const void* buf, size_t len, StaticSlice); + /// Copy constructor, adds a reference. Slice(const Slice& other); diff --git a/src/cpp/util/slice_cc.cc b/src/cpp/util/slice_cc.cc index 80b4b3a4712..56e0328b949 100644 --- a/src/cpp/util/slice_cc.cc +++ b/src/cpp/util/slice_cc.cc @@ -28,6 +28,19 @@ Slice::Slice(grpc_slice slice, AddRef) : slice_(grpc_slice_ref(slice)) {} Slice::Slice(grpc_slice slice, StealRef) : slice_(slice) {} +Slice::Slice(size_t len) : slice_(grpc_slice_malloc(len)) {} + +Slice::Slice(const void* buf, size_t len) + : slice_(grpc_slice_from_copied_buffer(reinterpret_cast(buf), + len)) {} + +Slice::Slice(const grpc::string& str) + : slice_(grpc_slice_from_copied_buffer(str.c_str(), str.length())) {} + +Slice::Slice(const void* buf, size_t len, StaticSlice) + : slice_(grpc_slice_from_static_buffer(reinterpret_cast(buf), + len)) {} + Slice::Slice(const Slice& other) : slice_(grpc_slice_ref(other.slice_)) {} } // namespace grpc diff --git a/test/cpp/qps/client.h b/test/cpp/qps/client.h index e759446c1ae..ecbf9c31fa1 100644 --- a/test/cpp/qps/client.h +++ b/test/cpp/qps/client.h @@ -88,9 +88,7 @@ class ClientRequestCreator { if (payload_config.has_bytebuf_params()) { std::unique_ptr buf( new char[payload_config.bytebuf_params().req_size()]); - grpc_slice s = grpc_slice_from_copied_buffer( - buf.get(), payload_config.bytebuf_params().req_size()); - Slice slice(s, Slice::STEAL_REF); + Slice slice(buf.get(), payload_config.bytebuf_params().req_size()); *req = ByteBuffer(&slice, 1); } else { GPR_ASSERT(false); // not appropriate for this specialization diff --git a/test/cpp/util/byte_buffer_proto_helper.cc b/test/cpp/util/byte_buffer_proto_helper.cc index 22dd074fe93..bb5162f86e6 100644 --- a/test/cpp/util/byte_buffer_proto_helper.cc +++ b/test/cpp/util/byte_buffer_proto_helper.cc @@ -36,8 +36,7 @@ std::unique_ptr SerializeToByteBuffer( grpc::protobuf::Message* message) { grpc::string buf; message->SerializeToString(&buf); - grpc_slice s = grpc_slice_from_copied_string(buf.c_str()); - Slice slice(s, Slice::STEAL_REF); + Slice slice(buf); return std::unique_ptr(new ByteBuffer(&slice, 1)); } diff --git a/test/cpp/util/byte_buffer_test.cc b/test/cpp/util/byte_buffer_test.cc index 6c077ddb2c3..cac01a73078 100644 --- a/test/cpp/util/byte_buffer_test.cc +++ b/test/cpp/util/byte_buffer_test.cc @@ -34,33 +34,30 @@ const char* kContent2 = "yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy world"; class ByteBufferTest : public ::testing::Test {}; TEST_F(ByteBufferTest, CreateFromSingleSlice) { - grpc_slice hello = grpc_slice_from_copied_string(kContent1); - Slice s(hello, Slice::STEAL_REF); + Slice s(kContent1); ByteBuffer buffer(&s, 1); + EXPECT_EQ(strlen(kContent1), buffer.Length()); } TEST_F(ByteBufferTest, CreateFromVector) { - grpc_slice hello = grpc_slice_from_copied_string(kContent1); - grpc_slice world = grpc_slice_from_copied_string(kContent2); std::vector slices; - slices.push_back(Slice(hello, Slice::STEAL_REF)); - slices.push_back(Slice(world, Slice::STEAL_REF)); + slices.emplace_back(kContent1); + slices.emplace_back(kContent2); ByteBuffer buffer(&slices[0], 2); + EXPECT_EQ(strlen(kContent1) + strlen(kContent2), buffer.Length()); } TEST_F(ByteBufferTest, Clear) { - grpc_slice hello = grpc_slice_from_copied_string(kContent1); - Slice s(hello, Slice::STEAL_REF); + Slice s(kContent1); ByteBuffer buffer(&s, 1); buffer.Clear(); + EXPECT_EQ(static_cast(0), buffer.Length()); } TEST_F(ByteBufferTest, Length) { - grpc_slice hello = grpc_slice_from_copied_string(kContent1); - grpc_slice world = grpc_slice_from_copied_string(kContent2); std::vector slices; - slices.push_back(Slice(hello, Slice::STEAL_REF)); - slices.push_back(Slice(world, Slice::STEAL_REF)); + slices.emplace_back(kContent1); + slices.emplace_back(kContent2); ByteBuffer buffer(&slices[0], 2); EXPECT_EQ(strlen(kContent1) + strlen(kContent2), buffer.Length()); } diff --git a/test/cpp/util/cli_call.cc b/test/cpp/util/cli_call.cc index fb6d76bb906..c3220efa544 100644 --- a/test/cpp/util/cli_call.cc +++ b/test/cpp/util/cli_call.cc @@ -119,8 +119,7 @@ void CliCall::WritesDone() { } void CliCall::WriteAndWait(const grpc::string& request) { - grpc_slice s = grpc_slice_from_copied_string(request.c_str()); - grpc::Slice req_slice(s, grpc::Slice::STEAL_REF); + grpc::Slice req_slice(request); grpc::ByteBuffer send_buffer(&req_slice, 1); gpr_mu_lock(&write_mu_); diff --git a/test/cpp/util/slice_test.cc b/test/cpp/util/slice_test.cc index 0f800035801..9e3329fab02 100644 --- a/test/cpp/util/slice_test.cc +++ b/test/cpp/util/slice_test.cc @@ -28,6 +28,9 @@ const char* kContent = "hello xxxxxxxxxxxxxxxxxxxx world"; class SliceTest : public ::testing::Test { protected: + void CheckSliceSize(const Slice& s, const grpc::string& content) { + EXPECT_EQ(content.size(), s.size()); + } void CheckSlice(const Slice& s, const grpc::string& content) { EXPECT_EQ(content.size(), s.size()); EXPECT_EQ(content, @@ -35,6 +38,31 @@ class SliceTest : public ::testing::Test { } }; +TEST_F(SliceTest, Empty) { + Slice empty_slice; + CheckSlice(empty_slice, ""); +} + +TEST_F(SliceTest, Sized) { + Slice sized_slice(strlen(kContent)); + CheckSliceSize(sized_slice, kContent); +} + +TEST_F(SliceTest, String) { + Slice spp(kContent); + CheckSlice(spp, kContent); +} + +TEST_F(SliceTest, Buf) { + Slice spp(kContent, strlen(kContent)); + CheckSlice(spp, kContent); +} + +TEST_F(SliceTest, StaticBuf) { + Slice spp(kContent, strlen(kContent), Slice::STATIC_SLICE); + CheckSlice(spp, kContent); +} + TEST_F(SliceTest, Steal) { grpc_slice s = grpc_slice_from_copied_string(kContent); Slice spp(s, Slice::STEAL_REF); @@ -48,11 +76,6 @@ TEST_F(SliceTest, Add) { CheckSlice(spp, kContent); } -TEST_F(SliceTest, Empty) { - Slice empty_slice; - CheckSlice(empty_slice, ""); -} - TEST_F(SliceTest, Cslice) { grpc_slice s = grpc_slice_from_copied_string(kContent); Slice spp(s, Slice::STEAL_REF);