protozero: make ParseVarInt more explicit in its return value
Instead of returning zero and the position which parsing got up to
before it finished if there are not enough bytes in the buffer,
return instead the start value of buffer if this is the case and
update ProtoDecoder to make use of this value.
Bug: 80416541
Change-Id: I4b7aa60c00a9a91f5f817dc25587ee8f8115a44a
diff --git a/src/protozero/proto_decoder.cc b/src/protozero/proto_decoder.cc
index 3f38a3c..eab565d 100644
--- a/src/protozero/proto_decoder.cc
+++ b/src/protozero/proto_decoder.cc
@@ -63,6 +63,7 @@
}
field.type = static_cast<FieldType>(raw_field_id & kFieldTypeMask);
+ const uint8_t* new_pos = nullptr;
uint64_t field_intvalue = 0;
switch (field.type) {
case kFieldTypeFixed64: {
@@ -85,45 +86,33 @@
break;
}
case kFieldTypeVarInt: {
- // We need to explicity check for zero to ensure that ParseVarInt doesn't
- // return zero because of running out of space in the buffer.
- if (*pos == 0) {
- pos++;
- field.int_value = 0;
- } else {
- pos = ParseVarInt(pos, end, &field.int_value);
+ new_pos = ParseVarInt(pos, end, &field.int_value);
- // The parsed value equalling zero means ParseVarInt could not fully
- // parse the number. This is because we are out of space in the buffer.
- // Set the id to zero and return but don't update the offset so a future
- // read can read this field.
- if (field.int_value == 0) {
- return field;
- }
+ // new_pos not being greater than pos means ParseVarInt could not fully
+ // parse the number. This is because we are out of space in the buffer.
+ // Set the id to zero and return but don't update the offset so a future
+ // read can read this field.
+ if (new_pos == pos) {
+ return field;
}
+ pos = new_pos;
break;
}
case kFieldTypeLengthDelimited: {
- // We need to explicity check for zero to ensure that ParseVarInt doesn't
- // return zero because of running out of space in the buffer.
- if (*pos == 0) {
- field.length_limited.data = ++pos;
- field.length_limited.length = 0;
- } else {
- pos = ParseVarInt(pos, end, &field_intvalue);
+ new_pos = ParseVarInt(pos, end, &field_intvalue);
- // The parsed value equalling zero means ParseVarInt could not fully
- // parse the number. This is because we are out of space in the buffer.
- // Alternatively, we may not have space to fully read the length
- // delimited field. Set the id to zero and return but don't update the
- // offset so a future read can read this field.
- if (field_intvalue == 0 || pos + field_intvalue > end) {
- return field;
- }
- field.length_limited.data = pos;
- field.length_limited.length = field_intvalue;
- pos += field_intvalue;
+ // new_pos not being greater than pos means ParseVarInt could not fully
+ // parse the number. This is because we are out of space in the buffer.
+ // Alternatively, we may not have space to fully read the length
+ // delimited field. Set the id to zero and return but don't update the
+ // offset so a future read can read this field.
+ if (new_pos == pos || pos + field_intvalue > end) {
+ return field;
}
+ pos = new_pos;
+ field.length_limited.data = pos;
+ field.length_limited.length = field_intvalue;
+ pos += field_intvalue;
break;
}
}
diff --git a/src/protozero/proto_utils.cc b/src/protozero/proto_utils.cc
index 6f1fba3..7a3798a 100644
--- a/src/protozero/proto_utils.cc
+++ b/src/protozero/proto_utils.cc
@@ -35,7 +35,7 @@
do {
if (PERFETTO_UNLIKELY(pos >= end)) {
*value = 0;
- break;
+ return start;
}
PERFETTO_DCHECK(shift < 64ull);
*value |= static_cast<uint64_t>(*pos & 0x7f) << shift;
diff --git a/src/protozero/proto_utils_unittest.cc b/src/protozero/proto_utils_unittest.cc
index 40296dd..3678b8f 100644
--- a/src/protozero/proto_utils_unittest.cc
+++ b/src/protozero/proto_utils_unittest.cc
@@ -161,7 +161,7 @@
for (size_t i = 0; i < 5; i++) {
uint64_t value = static_cast<uint64_t>(-1);
const uint8_t* res = ParseVarInt(buf, buf + i, &value);
- EXPECT_EQ(&buf[0] + i, res);
+ EXPECT_EQ(&buf[0], res);
EXPECT_EQ(0u, value);
}
}