Skip to content

Commit 75b66c2

Browse files
Prepare UnknownFieldSet for replace std::string with absl::string_view:
- Add overloads that take `absl::Cord` and `std::string&&` as inputs, putting the burden in the implementation instead of users. - Add overload that returns `absl::Span<char>` for callers that need higher performance requirements where we can avoid copies altogether. - Hide the APIs that return `std::string*` when the breaking change is enabled (via `-D PROTOBUF_TEMPORARY_ENABLE_STRING_VIEW_RETURN_TYPE`). PiperOrigin-RevId: 655600399
1 parent 165d2c7 commit 75b66c2

File tree

8 files changed

+217
-19
lines changed

8 files changed

+217
-19
lines changed

src/google/protobuf/arena_unittest.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@ TEST(ArenaTest, UnknownFields) {
652652
arena_message_3->mutable_unknown_fields()->AddVarint(1000, 42);
653653
arena_message_3->mutable_unknown_fields()->AddFixed32(1001, 42);
654654
arena_message_3->mutable_unknown_fields()->AddFixed64(1002, 42);
655-
arena_message_3->mutable_unknown_fields()->AddLengthDelimited(1003);
655+
arena_message_3->mutable_unknown_fields()->AddLengthDelimited(1003, "");
656656
arena_message_3->mutable_unknown_fields()->DeleteSubrange(0, 2);
657657
arena_message_3->mutable_unknown_fields()->DeleteByNumber(1002);
658658
arena_message_3->mutable_unknown_fields()->DeleteByNumber(1003);

src/google/protobuf/descriptor.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8921,11 +8921,12 @@ bool DescriptorBuilder::OptionInterpreter::InterpretSingleOption(
89218921
new UnknownFieldSet());
89228922
switch ((*iter)->type()) {
89238923
case FieldDescriptor::TYPE_MESSAGE: {
8924-
std::string* outstr =
8925-
parent_unknown_fields->AddLengthDelimited((*iter)->number());
8926-
ABSL_CHECK(unknown_fields->SerializeToString(outstr))
8924+
std::string outstr;
8925+
ABSL_CHECK(unknown_fields->SerializeToString(&outstr))
89278926
<< "Unexpected failure while serializing option submessage "
89288927
<< debug_msg_name << "\".";
8928+
parent_unknown_fields->AddLengthDelimited((*iter)->number(),
8929+
std::move(outstr));
89298930
break;
89308931
}
89318932

src/google/protobuf/parse_context.cc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include "absl/strings/cord.h"
1414
#include "absl/strings/string_view.h"
15+
#include "absl/types/span.h"
1516
#include "google/protobuf/message_lite.h"
1617
#include "google/protobuf/repeated_field.h"
1718
#include "google/protobuf/wire_format_lite.h"
@@ -265,6 +266,23 @@ const char* EpsCopyInputStream::ReadCordFallback(const char* ptr, int size,
265266
return ptr;
266267
}
267268

269+
const char* EpsCopyInputStream::ReadCharsFallback(const char* ptr,
270+
absl::Span<char> out) {
271+
char* out_ptr = out.data();
272+
ptr = AppendSize(ptr, out.size(), [&](const char* p, int s) {
273+
memcpy(out_ptr, p, s);
274+
out_ptr += s;
275+
});
276+
277+
// If we had an error, set the leftover memory to make sure we don't leak
278+
// uninit data in the object.
279+
if (ABSL_PREDICT_FALSE(ptr == nullptr)) {
280+
memset(out_ptr, 0xCD, out.data() + out.size() - out_ptr);
281+
}
282+
283+
return ptr;
284+
}
285+
268286

269287
const char* EpsCopyInputStream::InitFrom(io::ZeroCopyInputStream* zcis) {
270288
zcis_ = zcis;

src/google/protobuf/parse_context.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,15 @@ class PROTOBUF_EXPORT EpsCopyInputStream {
212212
return ReadCordFallback(ptr, size, cord);
213213
}
214214

215+
PROTOBUF_NODISCARD const char* ReadChars(const char* ptr,
216+
absl::Span<char> out) {
217+
if (out.size() <= static_cast<size_t>(buffer_end_ + kSlopBytes - ptr)) {
218+
memcpy(out.data(), ptr, out.size());
219+
return ptr + out.size();
220+
}
221+
return ReadCharsFallback(ptr, out);
222+
}
223+
215224

216225
template <typename Tag, typename T>
217226
PROTOBUF_NODISCARD const char* ReadRepeatedFixed(const char* ptr,
@@ -369,6 +378,7 @@ class PROTOBUF_EXPORT EpsCopyInputStream {
369378
const char* AppendStringFallback(const char* ptr, int size, std::string* str);
370379
const char* ReadStringFallback(const char* ptr, int size, std::string* str);
371380
const char* ReadCordFallback(const char* ptr, int size, absl::Cord* cord);
381+
const char* ReadCharsFallback(const char* ptr, absl::Span<char> out);
372382
static bool ParseEndsInSlopRegion(const char* begin, int overrun, int depth);
373383
bool StreamNext(const void** data) {
374384
bool res = zcis_->Next(data, &size_);

src/google/protobuf/unknown_field_set.cc

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,15 @@
1111

1212
#include "google/protobuf/unknown_field_set.h"
1313

14+
#include <cstring>
15+
#include <string>
16+
#include <utility>
17+
1418
#include "absl/log/absl_check.h"
1519
#include "absl/strings/cord.h"
1620
#include "absl/strings/internal/resize_uninitialized.h"
1721
#include "absl/strings/string_view.h"
22+
#include "absl/types/span.h"
1823
#include "google/protobuf/extension_set.h"
1924
#include "google/protobuf/generated_message_tctable_impl.h"
2025
#include "google/protobuf/io/coded_stream.h"
@@ -117,13 +122,44 @@ void UnknownFieldSet::AddFixed64(int number, uint64_t value) {
117122
field.data_.fixed64_ = value;
118123
}
119124

125+
void UnknownFieldSet::AddLengthDelimited(int number, const absl::Cord& value) {
126+
auto out = AddLengthDelimitedUninitialized(number, value.size());
127+
for (absl::string_view part : value.Chunks()) {
128+
memcpy(out.data(), part.data(), part.size());
129+
out.remove_prefix(part.size());
130+
}
131+
}
132+
133+
absl::Span<char> UnknownFieldSet::AddLengthDelimitedUninitialized(int number,
134+
size_t size) {
135+
auto& field = *fields_.Add();
136+
field.number_ = number;
137+
field.SetType(UnknownField::TYPE_LENGTH_DELIMITED);
138+
std::string* str = field.data_.string_value =
139+
Arena::Create<std::string>(arena());
140+
absl::strings_internal::STLStringResizeUninitialized(str, size);
141+
return absl::Span<char>(*str);
142+
}
143+
144+
template <int&...>
145+
void UnknownFieldSet::AddLengthDelimited(int number, std::string&& value) {
146+
auto& field = *fields_.Add();
147+
field.number_ = number;
148+
field.SetType(UnknownField::TYPE_LENGTH_DELIMITED);
149+
field.data_.string_value =
150+
Arena::Create<std::string>(arena(), std::move(value));
151+
}
152+
template void UnknownFieldSet::AddLengthDelimited(int, std::string&&);
153+
154+
#if !defined(PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE)
120155
std::string* UnknownFieldSet::AddLengthDelimited(int number) {
121156
auto& field = *fields_.Add();
122157
field.number_ = number;
123158
field.SetType(UnknownField::TYPE_LENGTH_DELIMITED);
124159
field.data_.string_value = Arena::Create<std::string>(arena());
125160
return field.data_.string_value;
126161
}
162+
#endif // PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE
127163

128164
UnknownFieldSet* UnknownFieldSet::AddGroup(int number) {
129165
auto& field = *fields_.Add();
@@ -284,10 +320,10 @@ class UnknownFieldParserHelper {
284320
}
285321
const char* ParseLengthDelimited(uint32_t num, const char* ptr,
286322
ParseContext* ctx) {
287-
std::string* s = unknown_->AddLengthDelimited(num);
288323
int size = ReadSize(&ptr);
289324
GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
290-
return ctx->ReadString(ptr, size, s);
325+
return ctx->ReadChars(ptr,
326+
unknown_->AddLengthDelimitedUninitialized(num, size));
291327
}
292328
const char* ParseGroup(uint32_t num, const char* ptr, ParseContext* ctx) {
293329
return ctx->ParseGroupInlined(ptr, num * 8 + 3, [&](const char* ptr) {

src/google/protobuf/unknown_field_set.h

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "absl/log/absl_check.h"
2525
#include "absl/strings/cord.h"
2626
#include "absl/strings/string_view.h"
27+
#include "absl/types/span.h"
2728
#include "google/protobuf/arena.h"
2829
#include "google/protobuf/io/coded_stream.h"
2930
#include "google/protobuf/io/zero_copy_stream_impl_lite.h"
@@ -47,6 +48,8 @@ class InternalMetadata; // metadata_lite.h
4748
class WireFormat; // wire_format.h
4849
class MessageSetFieldSkipperUsingCord;
4950
// extension_set_heavy.cc
51+
class UnknownFieldParserHelper;
52+
struct UnknownFieldSetTestPeer;
5053

5154
#if defined(PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE)
5255
using UFSStringView = absl::string_view;
@@ -87,7 +90,13 @@ class PROTOBUF_EXPORT UnknownField {
8790
inline void set_fixed32(uint32_t value);
8891
inline void set_fixed64(uint64_t value);
8992
inline void set_length_delimited(absl::string_view value);
93+
// template to avoid ambiguous overload resolution.
94+
template <int&...>
95+
inline void set_length_delimited(std::string&& value);
96+
inline void set_length_delimited(const absl::Cord& value);
97+
#if !defined(PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE)
9098
inline std::string* mutable_length_delimited();
99+
#endif // PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE
91100
inline UnknownFieldSet* mutable_group();
92101

93102
inline size_t GetLengthDelimitedSize() const;
@@ -191,7 +200,14 @@ class PROTOBUF_EXPORT UnknownFieldSet {
191200
void AddFixed32(int number, uint32_t value);
192201
void AddFixed64(int number, uint64_t value);
193202
void AddLengthDelimited(int number, absl::string_view value);
203+
// template to avoid ambiguous overload resolution.
204+
template <int&...>
205+
void AddLengthDelimited(int number, std::string&& value);
206+
void AddLengthDelimited(int number, const absl::Cord& value);
207+
208+
#if !defined(PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE)
194209
std::string* AddLengthDelimited(int number);
210+
#endif // PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE
195211
UnknownFieldSet* AddGroup(int number);
196212

197213
// Adds an unknown field from another set.
@@ -233,6 +249,10 @@ class PROTOBUF_EXPORT UnknownFieldSet {
233249
: UnknownFieldSet(arena) {}
234250

235251
private:
252+
friend internal::WireFormat;
253+
friend internal::UnknownFieldParserHelper;
254+
friend internal::UnknownFieldSetTestPeer;
255+
236256
using InternalArenaConstructable_ = void;
237257
using DestructorSkippable_ = void;
238258

@@ -241,6 +261,14 @@ class PROTOBUF_EXPORT UnknownFieldSet {
241261

242262
Arena* arena() { return fields_.GetArena(); }
243263

264+
// Returns a buffer of `size` chars for the user to fill in.
265+
// The buffer is potentially uninitialized memory. Failing to write to it
266+
// might lead to undefined behavior when reading it later.
267+
// Prefer the overloads above when possible. Calling this API without
268+
// validating the `size` parameter can lead to unintentional memory usage and
269+
// potential OOM.
270+
absl::Span<char> AddLengthDelimitedUninitialized(int number, size_t size);
271+
244272
void ClearFallback();
245273
void SwapSlow(UnknownFieldSet* other);
246274

@@ -275,7 +303,7 @@ inline void WriteVarint(uint32_t num, uint64_t val, UnknownFieldSet* unknown) {
275303
}
276304
inline void WriteLengthDelimited(uint32_t num, absl::string_view val,
277305
UnknownFieldSet* unknown) {
278-
unknown->AddLengthDelimited(num)->assign(val.data(), val.size());
306+
unknown->AddLengthDelimited(num, val);
279307
}
280308

281309
PROTOBUF_EXPORT
@@ -331,7 +359,10 @@ inline UnknownField* UnknownFieldSet::mutable_field(int index) {
331359

332360
inline void UnknownFieldSet::AddLengthDelimited(int number,
333361
const absl::string_view value) {
334-
AddLengthDelimited(number)->assign(value.data(), value.size());
362+
auto field = AddLengthDelimitedUninitialized(number, value.size());
363+
if (!value.empty()) {
364+
memcpy(field.data(), value.data(), value.size());
365+
}
335366
}
336367

337368
inline int UnknownField::number() const { return static_cast<int>(number_); }
@@ -376,10 +407,21 @@ inline void UnknownField::set_length_delimited(const absl::string_view value) {
376407
assert(type() == TYPE_LENGTH_DELIMITED);
377408
data_.string_value->assign(value.data(), value.size());
378409
}
410+
template <int&...>
411+
inline void UnknownField::set_length_delimited(std::string&& value) {
412+
assert(type() == TYPE_LENGTH_DELIMITED);
413+
*data_.string_value = std::move(value);
414+
}
415+
inline void UnknownField::set_length_delimited(const absl::Cord& value) {
416+
assert(type() == TYPE_LENGTH_DELIMITED);
417+
absl::CopyCordToString(value, data_.string_value);
418+
}
419+
#if !defined(PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE)
379420
inline std::string* UnknownField::mutable_length_delimited() {
380421
assert(type() == TYPE_LENGTH_DELIMITED);
381422
return data_.string_value;
382423
}
424+
#endif // PROTOBUF_FUTURE_STRING_VIEW_RETURN_TYPE
383425
inline UnknownFieldSet* UnknownField::mutable_group() {
384426
assert(type() == TYPE_GROUP);
385427
return data_.group_;
@@ -397,6 +439,8 @@ inline size_t UnknownField::GetLengthDelimitedSize() const {
397439

398440
inline void UnknownField::SetType(Type type) { type_ = type; }
399441

442+
extern template void UnknownFieldSet::AddLengthDelimited(int, std::string&&);
443+
400444
namespace internal {
401445

402446
// Add specialization of InternalMetadata::Container to provide arena support.

0 commit comments

Comments
 (0)