Support ERROR_ATOM_ID_INVALID_POSITION
Originally, the native implementation would discard everything clients
wrote to the buffer before the setAtomId call. Thus, the atom id was
always placed in the "correct position." However, for consistency with
the Java implementation, we now allow write calls before setAtomId to
occur and log an error in that case.
Test: bit libstatssocket_test:*
Bug: 152119205
Change-Id: Ib109c7a939a5ae92c1d5fc70aa647ac7390fadad
diff --git a/libstats/socket/include/stats_event.h b/libstats/socket/include/stats_event.h
index 3576298..00dc76e 100644
--- a/libstats/socket/include/stats_event.h
+++ b/libstats/socket/include/stats_event.h
@@ -89,7 +89,8 @@
/**
* Sets the atom id for this StatsEvent.
*
- * This function should be called immediately after AStatsEvent_obtain.
+ * This function should be called immediately after AStatsEvent_obtain. It may
+ * be called additional times as well, but subsequent calls will have no effect.
**/
void AStatsEvent_setAtomId(AStatsEvent* event, uint32_t atomId);
diff --git a/libstats/socket/stats_event.c b/libstats/socket/stats_event.c
index 24d2ea8..e63bc07 100644
--- a/libstats/socket/stats_event.c
+++ b/libstats/socket/stats_event.c
@@ -47,6 +47,7 @@
#define ERROR_TOO_MANY_FIELDS 0x200
#define ERROR_INVALID_VALUE_TYPE 0x400
#define ERROR_STRING_NOT_NULL_TERMINATED 0x800
+#define ERROR_ATOM_ID_INVALID_POSITION 0x2000
/* TYPE IDS */
#define INT32_TYPE 0x00
@@ -98,11 +99,6 @@
event->buf[0] = OBJECT_TYPE;
AStatsEvent_writeInt64(event, get_elapsed_realtime_ns()); // write the timestamp
- // Force client to set atom id immediately (this is required for atom-level
- // annotations to be written correctly). All atom field and annotation
- // writes will fail until the atom id is set because event->errors != 0.
- event->errors |= ERROR_NO_ATOM_ID;
-
return event;
}
@@ -112,10 +108,12 @@
}
void AStatsEvent_setAtomId(AStatsEvent* event, uint32_t atomId) {
- if ((event->errors & ERROR_NO_ATOM_ID) == 0) return;
+ if (event->atomId != 0) return;
+ if (event->numElements != 1) {
+ event->errors |= ERROR_ATOM_ID_INVALID_POSITION;
+ return;
+ }
- // Clear the ERROR_NO_ATOM_ID bit.
- event->errors &= ~ERROR_NO_ATOM_ID;
event->atomId = atomId;
AStatsEvent_writeInt32(event, atomId);
}
@@ -197,36 +195,26 @@
}
void AStatsEvent_writeInt32(AStatsEvent* event, int32_t value) {
- if (event->errors) return;
-
start_field(event, INT32_TYPE);
append_int32(event, value);
}
void AStatsEvent_writeInt64(AStatsEvent* event, int64_t value) {
- if (event->errors) return;
-
start_field(event, INT64_TYPE);
append_int64(event, value);
}
void AStatsEvent_writeFloat(AStatsEvent* event, float value) {
- if (event->errors) return;
-
start_field(event, FLOAT_TYPE);
append_float(event, value);
}
void AStatsEvent_writeBool(AStatsEvent* event, bool value) {
- if (event->errors) return;
-
start_field(event, BOOL_TYPE);
append_bool(event, value);
}
void AStatsEvent_writeByteArray(AStatsEvent* event, const uint8_t* buf, size_t numBytes) {
- if (event->errors) return;
-
start_field(event, BYTE_ARRAY_TYPE);
append_int32(event, numBytes);
append_byte_array(event, buf, numBytes);
@@ -234,8 +222,6 @@
// Value is assumed to be encoded using UTF8
void AStatsEvent_writeString(AStatsEvent* event, const char* value) {
- if (event->errors) return;
-
start_field(event, STRING_TYPE);
append_string(event, value);
}
@@ -243,8 +229,10 @@
// Tags are assumed to be encoded using UTF8
void AStatsEvent_writeAttributionChain(AStatsEvent* event, const uint32_t* uids,
const char* const* tags, uint8_t numNodes) {
- if (numNodes > MAX_BYTE_VALUE) event->errors |= ERROR_ATTRIBUTION_CHAIN_TOO_LONG;
- if (event->errors) return;
+ if (numNodes > MAX_BYTE_VALUE) {
+ event->errors |= ERROR_ATTRIBUTION_CHAIN_TOO_LONG;
+ return;
+ }
start_field(event, ATTRIBUTION_CHAIN_TYPE);
append_byte(event, numNodes);
@@ -270,9 +258,13 @@
}
void AStatsEvent_addBoolAnnotation(AStatsEvent* event, uint8_t annotationId, bool value) {
- if (event->lastFieldPos == 0) event->errors |= ERROR_ANNOTATION_DOES_NOT_FOLLOW_FIELD;
- if (annotationId > MAX_BYTE_VALUE) event->errors |= ERROR_ANNOTATION_ID_TOO_LARGE;
- if (event->errors) return;
+ if (event->numElements < 2) {
+ event->errors |= ERROR_ANNOTATION_DOES_NOT_FOLLOW_FIELD;
+ return;
+ } else if (annotationId > MAX_BYTE_VALUE) {
+ event->errors |= ERROR_ANNOTATION_ID_TOO_LARGE;
+ return;
+ }
append_byte(event, annotationId);
append_byte(event, BOOL_TYPE);
@@ -281,9 +273,13 @@
}
void AStatsEvent_addInt32Annotation(AStatsEvent* event, uint8_t annotationId, int32_t value) {
- if (event->lastFieldPos == 0) event->errors |= ERROR_ANNOTATION_DOES_NOT_FOLLOW_FIELD;
- if (annotationId > MAX_BYTE_VALUE) event->errors |= ERROR_ANNOTATION_ID_TOO_LARGE;
- if (event->errors) return;
+ if (event->numElements < 2) {
+ event->errors |= ERROR_ANNOTATION_DOES_NOT_FOLLOW_FIELD;
+ return;
+ } else if (annotationId > MAX_BYTE_VALUE) {
+ event->errors |= ERROR_ANNOTATION_ID_TOO_LARGE;
+ return;
+ }
append_byte(event, annotationId);
append_byte(event, INT32_TYPE);
diff --git a/libstats/socket/tests/stats_event_test.cpp b/libstats/socket/tests/stats_event_test.cpp
index 04eff36..6e47e3d 100644
--- a/libstats/socket/tests/stats_event_test.cpp
+++ b/libstats/socket/tests/stats_event_test.cpp
@@ -32,6 +32,7 @@
#define ERROR_TOO_MANY_FIELDS 0x200
#define ERROR_INVALID_VALUE_TYPE 0x400
#define ERROR_STRING_NOT_NULL_TERMINATED 0x800
+#define ERROR_ATOM_ID_INVALID_POSITION 0x2000
/* TYPE IDS */
#define INT32_TYPE 0x00
@@ -358,6 +359,19 @@
AStatsEvent_release(event);
}
+TEST(StatsEventTest, TestAtomIdInvalidPositionError) {
+ AStatsEvent* event = AStatsEvent_obtain();
+ AStatsEvent_writeInt32(event, 0);
+ AStatsEvent_setAtomId(event, 100);
+ AStatsEvent_writeBool(event, true);
+ AStatsEvent_build(event);
+
+ uint32_t errors = AStatsEvent_getErrors(event);
+ EXPECT_NE(errors | ERROR_ATOM_ID_INVALID_POSITION, 0);
+
+ AStatsEvent_release(event);
+}
+
TEST(StatsEventTest, TestOverwriteTimestamp) {
uint32_t atomId = 100;
int64_t expectedTimestamp = 0x123456789;