Thrift: Clean up and test TDenseProtocol
Summary:
- TDenseProtocol now includes a part of the struct fingerprint in
the serialized message, to protect from unserialzing trash.
- A lot of cleanups and commenting for TDenseProtocol.
- A lot of test cases for same.
Reviewed By: mcslee
Test Plan: test/DenseProtoTest.cpp
Revert Plan: ok
git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@665257 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/lib/cpp/src/TReflectionLocal.h b/lib/cpp/src/TReflectionLocal.h
index df6e706..3fe7c63 100644
--- a/lib/cpp/src/TReflectionLocal.h
+++ b/lib/cpp/src/TReflectionLocal.h
@@ -8,18 +8,25 @@
#define _THRIFT_TREFLECTIONLOCAL_H_ 1
#include <stdint.h>
+#include <cstring>
#include <protocol/TProtocol.h>
+/**
+ * Local Reflection is a blanket term referring to the the structure
+ * and generation of this particular representation of Thrift types.
+ * (It is called local because it cannot be serialized by Thrift).
+ *
+ * @author David Reiss <dreiss@facebook.com>
+ */
+
namespace facebook { namespace thrift { namespace reflection { namespace local {
using facebook::thrift::protocol::TType;
-/**
- * A local reflection is a representation of a Thrift structure.
- * (It is called local because it cannot be serialized by Thrift).
- *
- * @author David Reiss <dreiss@facebook.com>
- */
+// We include this many bytes of the structure's fingerprint when serializing
+// a top-level structure. Long enough to make collisions unlikely, short
+// enough to not significantly affect the amount of memory used.
+const int FP_PREFIX_LEN = 4;
struct FieldMeta {
int16_t tag;
@@ -27,16 +34,15 @@
};
struct TypeSpec {
+ TType ttype;
+ uint8_t fp_prefix[FP_PREFIX_LEN];
+
// Use an anonymous union here so we can fit two TypeSpecs in one cache line.
union {
struct {
// Use parallel arrays here for denser packing (of the arrays).
FieldMeta* metas;
TypeSpec** specs;
- // n_fields is only used for debugging, but it should only add
- // a minimimal amount to the effective size of this structure
- // because of alignment restrictions.
- int n_fields;
} tstruct;
struct {
TypeSpec *subtype1;
@@ -44,21 +50,21 @@
} tcontainer;
};
- // Put this at the end so the 32-bit enum can be crammed up next to the
- // 32-bit int (n_fields).
- TType ttype;
-
-
// Static initialization of unions isn't really possible,
// so take the plunge and use constructors.
// Hopefully they'll be evaluated at compile time.
- TypeSpec(TType ttype) : ttype(ttype) {}
+ TypeSpec(TType ttype) : ttype(ttype) {
+ std::memset(fp_prefix, 0, FP_PREFIX_LEN);
+ }
- TypeSpec(TType ttype, int n_fields, FieldMeta* metas, TypeSpec** specs) :
+ TypeSpec(TType ttype,
+ uint8_t* fingerprint,
+ FieldMeta* metas,
+ TypeSpec** specs) :
ttype(ttype)
{
- tstruct.n_fields = n_fields;
+ std::memcpy(fp_prefix, fingerprint, FP_PREFIX_LEN);
tstruct.metas = metas;
tstruct.specs = specs;
}
@@ -66,6 +72,7 @@
TypeSpec(TType ttype, TypeSpec* subtype1, TypeSpec* subtype2) :
ttype(ttype)
{
+ std::memset(fp_prefix, 0, FP_PREFIX_LEN);
tcontainer.subtype1 = subtype1;
tcontainer.subtype2 = subtype2;
}