Skip to content

Commit d55b54e

Browse files
Pranav Thulasiram Bhatfacebook-github-bot
Pranav Thulasiram Bhat
authored andcommitted
Require names to be null-terminated
Summary: It is conveinent to cast these to c-style strings (e.g. to pass into DebugProtocol::writeFieldBegin) This diff adds a requirement that names contained within SyntaxGraph are null-terminated. Reviewed By: iahs Differential Revision: D73053809 fbshipit-source-id: 8e4eb9a839338849ba9e74f933d965695a40f8b4
1 parent 5207170 commit d55b54e

File tree

3 files changed

+14
-1
lines changed

3 files changed

+14
-1
lines changed

thrift/lib/cpp2/schema/SyntaxGraph.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,12 @@ class DefinitionKeyEqual : public std::equal_to<type::DefinitionKey> {
8484

8585
} // namespace
8686

87+
WithName::WithName(std::string_view name) : name_(name) {
88+
FOLLY_SAFE_DCHECK(
89+
name_.data()[name_.size()] == '\0',
90+
"name must be backed by a null-terminated string!");
91+
}
92+
8793
class Resolver final {
8894
private:
8995
friend class ::apache::thrift::schema::SyntaxGraph;

thrift/lib/cpp2/schema/SyntaxGraph.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -387,10 +387,13 @@ class Lazy {
387387
*/
388388
class WithName {
389389
protected:
390-
explicit WithName(std::string_view name) : name_(name) {}
390+
// Requirement: `name` is backed by a null-terminated string (e.g. static
391+
// string or std::string)
392+
explicit WithName(std::string_view name);
391393

392394
/**
393395
* The scoped name of this graph node.
396+
* Can be assumed to be null-terminated
394397
*/
395398
std::string_view name() const { return name_; }
396399

thrift/lib/cpp2/schema/test/SyntaxGraphTest.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ TEST_F(ServiceSchemaTest, Struct) {
173173
EXPECT_EQ(&testStruct->program(), program.unwrap());
174174
EXPECT_EQ(testStruct->kind(), DefinitionNode::Kind::STRUCT);
175175
EXPECT_EQ(testStruct->name(), "TestStruct");
176+
EXPECT_STREQ(testStruct->name().data(), "TestStruct");
176177
const StructNode& s = testStruct->asStruct();
177178
EXPECT_EQ(&s.definition(), testStruct.unwrap());
178179
EXPECT_EQ(s.uri(), "");
@@ -184,6 +185,7 @@ TEST_F(ServiceSchemaTest, Struct) {
184185
s.fields()[0].presence(), FieldNode::PresenceQualifier::UNQUALIFIED);
185186
EXPECT_EQ(s.fields()[0].type().asPrimitive(), Primitive::I32);
186187
EXPECT_EQ(s.fields()[0].name(), "field1");
188+
EXPECT_STREQ(s.fields()[0].name().data(), "field1");
187189
EXPECT_EQ(s.fields()[0].customDefault()->as_i32(), 10);
188190

189191
EXPECT_EQ(s.fields()[1].id(), FieldId{2});
@@ -192,6 +194,7 @@ TEST_F(ServiceSchemaTest, Struct) {
192194
&s.fields()[1].type().asEnum(),
193195
&program->definitionsByName().at("TestEnum")->asEnum());
194196
EXPECT_EQ(s.fields()[1].name(), "field2");
197+
EXPECT_STREQ(s.fields()[1].name().data(), "field2");
195198
EXPECT_EQ(s.fields()[1].customDefault(), nullptr);
196199
}
197200

@@ -204,6 +207,7 @@ TEST_F(ServiceSchemaTest, Union) {
204207
EXPECT_EQ(&testUnion->program(), program.unwrap());
205208
EXPECT_EQ(testUnion->kind(), DefinitionNode::Kind::UNION);
206209
EXPECT_EQ(testUnion->name(), "TestUnion");
210+
EXPECT_STREQ(testUnion->name().data(), "TestUnion");
207211
const UnionNode& u = testUnion->asUnion();
208212

209213
EXPECT_EQ(u.fields().size(), 2);

0 commit comments

Comments
 (0)