Code Review Feedback: Remove access to Value in MetricEvent to avoid …

…incorrect access
anush-apple · Feb 23, 2024 · b5d7d37 · b5d7d37
1 parent 4b48de7
commit b5d7d37
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 7 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRMetricsCollector.mm b/src/darwin/Framework/CHIP/MTRMetricsCollector.mm
@@ -49,7 +49,7 @@ - (instancetype)initWithMetricEvent:(const MetricEvent &)event
     }
 
     using ValueType = MetricEvent::Value::Type;
-    switch (event.value().type) {
+    switch (event.ValueType()) {
     case ValueType::kInt32:
         _value = [NSNumber numberWithInteger:event.ValueInt32()];
         break;
@@ -163,7 +163,7 @@ - (void)handleMetricEvent:(MetricEvent)event
     std::lock_guard lock(_lock);
 
     using ValueType = MetricEvent::Value::Type;
-    switch (event.value().type) {
+    switch (event.ValueType()) {
     case ValueType::kInt32:
         MTR_LOG_INFO("Received metric event, key: %s, type: %d, value: %d", event.key(), event.type(), event.ValueInt32());
         break;
@@ -201,7 +201,7 @@ - (void)handleMetricEvent:(MetricEvent)event
 
     // If the event is a begin or end event, implicitly emit a corresponding instant event
     if (event.type() == MetricEvent::Type::kBeginEvent || event.type() == MetricEvent::Type::kEndEvent) {
-        MetricEvent instantEvent(MetricEvent::Type::kInstantEvent, event.key(), event.value());
+        MetricEvent instantEvent(MetricEvent::Type::kInstantEvent, event.key());
         data = [[MTRMetricsData alloc] initWithMetricEvent:instantEvent];
         metricsKey = [NSString stringWithFormat:@"%s%@", event.key(), suffixNameForMetric(instantEvent)];
         [_metricsDataCollection setValue:data forKey:metricsKey];

diff --git a/src/tracing/esp32_trace/esp32_tracing.cpp b/src/tracing/esp32_trace/esp32_tracing.cpp
@@ -166,7 +166,7 @@ void ESP32Backend::LogMetricEvent(const MetricEvent & event)
     }
 
     using ValueType = MetricEvent::Value::Type;
-    switch (event.value().type)
+    switch (event.ValueType())
     {
     case ValueType::kInt32:
         ESP_LOGI("mtr", "The value of %s is %ld ", event.key(), event.ValueInt32());

diff --git a/src/tracing/json/json_tracing.cpp b/src/tracing/json/json_tracing.cpp
@@ -303,7 +303,7 @@ void JsonBackend::LogMetricEvent(const MetricEvent & event)
     value["label"] = event.key();
 
     using ValueType = MetricEvent::Value::Type;
-    switch (event.value().type)
+    switch (event.ValueType())
     {
     case ValueType::kInt32:
         value["value"] = event.ValueInt32();

diff --git a/src/tracing/metric_event.h b/src/tracing/metric_event.h
@@ -117,7 +117,10 @@ class MetricEvent
 
     MetricKey key() const { return mKey; }
 
-    Value value() const { return mValue; }
+    Value::Type ValueType() const
+    {
+        return mValue.type;
+    }
 
     uint32_t ValueUInt32() const
     {

diff --git a/src/tracing/perfetto/perfetto_tracing.cpp b/src/tracing/perfetto/perfetto_tracing.cpp
@@ -130,7 +130,7 @@ void PerfettoBackend::LogNodeDiscoveryFailed(NodeDiscoveryFailedInfo & info)
 void PerfettoBackend::LogMetricEvent(const MetricEvent & event)
 {
     using ValueType = MetricEvent::Value::Type;
-    switch (event.value().type)
+    switch (event.ValueType())
     {
     case ValueType::kInt32:
         TRACE_EVENT_INSTANT("Matter", event.key(), "value", event.ValueInt32());