MediaMetrics: Use variant instead of union for properties

Test: Verify media.metrics dumpsys, atest mediametrics_tests
Bug: 138583596
Change-Id: Ic6e2a090b890f5bec1e818637f6e31150924add1
diff --git a/media/libmediametrics/MediaMetricsItem.cpp b/media/libmediametrics/MediaMetricsItem.cpp
index 485e161..62af0f7 100644
--- a/media/libmediametrics/MediaMetricsItem.cpp
+++ b/media/libmediametrics/MediaMetricsItem.cpp
@@ -231,7 +231,7 @@
             mPkgName.c_str(), mProps.size());
     result.append(buffer);
     for (auto &prop : *this) {
-        prop.toString(buffer, sizeof(buffer));
+        prop.toStringBuffer(buffer, sizeof(buffer));
         result.append(buffer);
     }
     result.append("]");
@@ -364,74 +364,6 @@
 }
 
 
-namespace {
-
-template <typename T>
-status_t insert(const T& val, char **bufferpptr, char *bufferptrmax)
-{
-    const size_t size = sizeof(val);
-    if (*bufferpptr + size > bufferptrmax) {
-        ALOGE("%s: buffer exceeded with size %zu", __func__, size);
-        return BAD_VALUE;
-    }
-    memcpy(*bufferpptr, &val, size);
-    *bufferpptr += size;
-    return NO_ERROR;
-}
-
-template <>
-status_t insert(const char * const& val, char **bufferpptr, char *bufferptrmax)
-{
-    const size_t size = strlen(val) + 1;
-    if (size > UINT16_MAX || *bufferpptr + size > bufferptrmax) {
-        ALOGE("%s: buffer exceeded with size %zu", __func__, size);
-        return BAD_VALUE;
-    }
-    memcpy(*bufferpptr, val, size);
-    *bufferpptr += size;
-    return NO_ERROR;
-}
-
-template <>
- __unused
-status_t insert(char * const& val, char **bufferpptr, char *bufferptrmax)
-{
-    return insert((const char *)val, bufferpptr, bufferptrmax);
-}
-
-template <typename T>
-status_t extract(T *val, const char **bufferpptr, const char *bufferptrmax)
-{
-    const size_t size = sizeof(*val);
-    if (*bufferpptr + size > bufferptrmax) {
-        ALOGE("%s: buffer exceeded with size %zu", __func__, size);
-        return BAD_VALUE;
-    }
-    memcpy(val, *bufferpptr, size);
-    *bufferpptr += size;
-    return NO_ERROR;
-}
-
-template <>
-status_t extract(char **val, const char **bufferpptr, const char *bufferptrmax)
-{
-    const char *ptr = *bufferpptr;
-    while (*ptr != 0) {
-        if (ptr >= bufferptrmax) {
-            ALOGE("%s: buffer exceeded", __func__);
-            return BAD_VALUE;
-        }
-        ++ptr;
-    }
-    const size_t size = (ptr - *bufferpptr) + 1;
-    *val = (char *)malloc(size);
-    memcpy(*val, *bufferpptr, size);
-    *bufferpptr += size;
-    return NO_ERROR;
-}
-
-} // namespace
-
 status_t mediametrics::Item::writeToByteString(char **pbuffer, size_t *plength) const
 {
     if (pbuffer == nullptr || plength == nullptr)
@@ -521,7 +453,7 @@
     uint32_t header_size;
     uint16_t version;
     uint16_t key_size;
-    char *key = nullptr;
+    std::string key;
     int32_t pid;
     int32_t uid;
     int64_t timestamp;
@@ -535,14 +467,12 @@
             || extract(&uid, &read, readend) != NO_ERROR
             || extract(&timestamp, &read, readend) != NO_ERROR
             || size > length
-            || strlen(key) + 1 != key_size
+            || key.size() + 1 != key_size
             || header_size > size) {
-        free(key);
         ALOGW("%s: invalid header", __func__);
         return INVALID_OPERATION;
     }
-    mKey = key;
-    free(key);
+    mKey = std::move(key);
     const size_t pos = read - bufferptr;
     if (pos > header_size) {
         ALOGW("%s: invalid header pos:%zu > header_size:%u",
@@ -571,36 +501,6 @@
     return NO_ERROR;
 }
 
-status_t mediametrics::Item::Prop::writeToParcel(Parcel *data) const
-{
-   switch (mType) {
-   case mediametrics::kTypeInt32:
-       return data->writeCString(mName.c_str())
-               ?: data->writeInt32(mType)
-               ?: data->writeInt32(u.int32Value);
-   case mediametrics::kTypeInt64:
-       return data->writeCString(mName.c_str())
-               ?: data->writeInt32(mType)
-               ?: data->writeInt64(u.int64Value);
-   case mediametrics::kTypeDouble:
-       return data->writeCString(mName.c_str())
-               ?: data->writeInt32(mType)
-               ?: data->writeDouble(u.doubleValue);
-   case mediametrics::kTypeRate:
-       return data->writeCString(mName.c_str())
-               ?: data->writeInt32(mType)
-               ?: data->writeInt64(u.rate.first)
-               ?: data->writeInt64(u.rate.second);
-   case mediametrics::kTypeCString:
-       return data->writeCString(mName.c_str())
-               ?: data->writeInt32(mType)
-               ?: data->writeCString(u.CStringValue);
-   default:
-       ALOGE("%s: found bad type: %d, name %s", __func__, mType, mName.c_str());
-       return BAD_VALUE;
-   }
-}
-
 status_t mediametrics::Item::Prop::readFromParcel(const Parcel& data)
 {
     const char *key = data.readCString();
@@ -609,237 +509,99 @@
     status_t status = data.readInt32(&type);
     if (status != NO_ERROR) return status;
     switch (type) {
-    case mediametrics::kTypeInt32:
-        status = data.readInt32(&u.int32Value);
-        break;
-    case mediametrics::kTypeInt64:
-        status = data.readInt64(&u.int64Value);
-        break;
-    case mediametrics::kTypeDouble:
-        status = data.readDouble(&u.doubleValue);
-        break;
+    case mediametrics::kTypeInt32: {
+        int32_t value;
+        status = data.readInt32(&value);
+        if (status != NO_ERROR) return status;
+        mElem = value;
+    } break;
+    case mediametrics::kTypeInt64: {
+        int64_t value;
+        status = data.readInt64(&value);
+        if (status != NO_ERROR) return status;
+        mElem = value;
+    } break;
+    case mediametrics::kTypeDouble: {
+        double value;
+        status = data.readDouble(&value);
+        if (status != NO_ERROR) return status;
+        mElem = value;
+    } break;
     case mediametrics::kTypeCString: {
         const char *s = data.readCString();
         if (s == nullptr) return BAD_VALUE;
-        set(s);
-        break;
-        }
+        mElem = s;
+    } break;
     case mediametrics::kTypeRate: {
         std::pair<int64_t, int64_t> rate;
         status = data.readInt64(&rate.first)
                 ?: data.readInt64(&rate.second);
-        if (status == NO_ERROR) {
-            set(rate);
-        }
-        break;
-        }
+        if (status != NO_ERROR) return status;
+        mElem = rate;
+    } break;
+    case mediametrics::kTypeNone: {
+        mElem = std::monostate{};
+    } break;
     default:
-        ALOGE("%s: reading bad item type: %d", __func__, mType);
+        ALOGE("%s: reading bad item type: %d", __func__, type);
         return BAD_VALUE;
     }
-    if (status == NO_ERROR) {
-        setName(key);
-        mType = (mediametrics::Type)type;
-    }
-    return status;
-}
-
-void mediametrics::Item::Prop::toString(char *buffer, size_t length) const
-{
-    switch (mType) {
-    case mediametrics::kTypeInt32:
-        snprintf(buffer, length, "%s=%d:", mName.c_str(), u.int32Value);
-        break;
-    case mediametrics::kTypeInt64:
-        snprintf(buffer, length, "%s=%lld:", mName.c_str(), (long long)u.int64Value);
-        break;
-    case mediametrics::kTypeDouble:
-        snprintf(buffer, length, "%s=%e:", mName.c_str(), u.doubleValue);
-        break;
-    case mediametrics::kTypeRate:
-        snprintf(buffer, length, "%s=%lld/%lld:",
-                mName.c_str(), (long long)u.rate.first, (long long)u.rate.second);
-        break;
-    case mediametrics::kTypeCString:
-        // TODO sanitize string for ':' '='
-        snprintf(buffer, length, "%s=%s:", mName.c_str(), u.CStringValue);
-        break;
-    default:
-        ALOGE("%s: bad item type: %d for %s", __func__, mType, mName.c_str());
-        if (length > 0) buffer[0] = 0;
-        break;
-    }
-}
-
-size_t mediametrics::Item::Prop::getByteStringSize() const
-{
-    const size_t header =
-        sizeof(uint16_t)      // length
-        + sizeof(uint8_t)     // type
-        + mName.size() + 1;  // mName + 0 termination
-    size_t payload = 0;
-    switch (mType) {
-    case mediametrics::kTypeInt32:
-        payload = sizeof(u.int32Value);
-        break;
-    case mediametrics::kTypeInt64:
-        payload = sizeof(u.int64Value);
-        break;
-    case mediametrics::kTypeDouble:
-        payload = sizeof(u.doubleValue);
-        break;
-    case mediametrics::kTypeRate:
-        payload = sizeof(u.rate.first) + sizeof(u.rate.second);
-        break;
-    case mediametrics::kTypeCString:
-        payload = strlen(u.CStringValue) + 1;
-        break;
-    default:
-        ALOGE("%s: found bad prop type: %d, name %s",
-                __func__, mType, mName.c_str()); // no payload computed
-        break;
-    }
-    return header + payload;
-}
-
-
-// TODO: fold into a template later.
-status_t BaseItem::writeToByteString(
-        const char *name, int32_t value, char **bufferpptr, char *bufferptrmax)
-{
-    const size_t len = 2 + 1 + strlen(name) + 1 + sizeof(value);
-    if (len > UINT16_MAX) return BAD_VALUE;
-    return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)mediametrics::kTypeInt32, bufferpptr, bufferptrmax)
-            ?: insert(name, bufferpptr, bufferptrmax)
-            ?: insert(value, bufferpptr, bufferptrmax);
-}
-
-status_t BaseItem::writeToByteString(
-        const char *name, int64_t value, char **bufferpptr, char *bufferptrmax)
-{
-    const size_t len = 2 + 1 + strlen(name) + 1 + sizeof(value);
-    if (len > UINT16_MAX) return BAD_VALUE;
-    return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)mediametrics::kTypeInt64, bufferpptr, bufferptrmax)
-            ?: insert(name, bufferpptr, bufferptrmax)
-            ?: insert(value, bufferpptr, bufferptrmax);
-}
-
-status_t BaseItem::writeToByteString(
-        const char *name, double value, char **bufferpptr, char *bufferptrmax)
-{
-    const size_t len = 2 + 1 + strlen(name) + 1 + sizeof(value);
-    if (len > UINT16_MAX) return BAD_VALUE;
-    return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)mediametrics::kTypeDouble, bufferpptr, bufferptrmax)
-            ?: insert(name, bufferpptr, bufferptrmax)
-            ?: insert(value, bufferpptr, bufferptrmax);
-}
-
-status_t BaseItem::writeToByteString(
-        const char *name, const std::pair<int64_t, int64_t> &value, char **bufferpptr, char *bufferptrmax)
-{
-    const size_t len = 2 + 1 + strlen(name) + 1 + 8 + 8;
-    if (len > UINT16_MAX) return BAD_VALUE;
-    return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)mediametrics::kTypeRate, bufferpptr, bufferptrmax)
-            ?: insert(name, bufferpptr, bufferptrmax)
-            ?: insert(value.first, bufferpptr, bufferptrmax)
-            ?: insert(value.second, bufferpptr, bufferptrmax);
-}
-
-status_t BaseItem::writeToByteString(
-        const char *name, char * const &value, char **bufferpptr, char *bufferptrmax)
-{
-    return writeToByteString(name, (const char *)value, bufferpptr, bufferptrmax);
-}
-
-status_t BaseItem::writeToByteString(
-        const char *name, const char * const &value, char **bufferpptr, char *bufferptrmax)
-{
-    const size_t len = 2 + 1 + strlen(name) + 1 + strlen(value) + 1;
-    if (len > UINT16_MAX) return BAD_VALUE;
-    return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)mediametrics::kTypeCString, bufferpptr, bufferptrmax)
-            ?: insert(name, bufferpptr, bufferptrmax)
-            ?: insert(value, bufferpptr, bufferptrmax);
-}
-
-
-status_t BaseItem::writeToByteString(
-        const char *name, const none_t &, char **bufferpptr, char *bufferptrmax)
-{
-    const size_t len = 2 + 1 + strlen(name) + 1;
-    if (len > UINT16_MAX) return BAD_VALUE;
-    return insert((uint16_t)len, bufferpptr, bufferptrmax)
-            ?: insert((uint8_t)mediametrics::kTypeCString, bufferpptr, bufferptrmax)
-            ?: insert(name, bufferpptr, bufferptrmax);
-}
-
-
-status_t mediametrics::Item::Prop::writeToByteString(
-        char **bufferpptr, char *bufferptrmax) const
-{
-    switch (mType) {
-    case mediametrics::kTypeInt32:
-        return BaseItem::writeToByteString(mName.c_str(), u.int32Value, bufferpptr, bufferptrmax);
-    case mediametrics::kTypeInt64:
-        return BaseItem::writeToByteString(mName.c_str(), u.int64Value, bufferpptr, bufferptrmax);
-    case mediametrics::kTypeDouble:
-        return BaseItem::writeToByteString(mName.c_str(), u.doubleValue, bufferpptr, bufferptrmax);
-    case mediametrics::kTypeRate:
-        return BaseItem::writeToByteString(mName.c_str(), u.rate, bufferpptr, bufferptrmax);
-    case mediametrics::kTypeCString:
-        return BaseItem::writeToByteString(
-                mName.c_str(), u.CStringValue, bufferpptr, bufferptrmax);
-    case mediametrics::kTypeNone:
-        return BaseItem::writeToByteString(mName.c_str(), none_t{}, bufferpptr, bufferptrmax);
-    default:
-        ALOGE("%s: found bad prop type: %d, name %s",
-                __func__, mType, mName.c_str());  // no payload sent
-        return BAD_VALUE;
-    }
+    setName(key);
+    return NO_ERROR;
 }
 
 status_t mediametrics::Item::Prop::readFromByteString(
         const char **bufferpptr, const char *bufferptrmax)
 {
     uint16_t len;
-    char *name;
+    std::string name;
     uint8_t type;
     status_t status = extract(&len, bufferpptr, bufferptrmax)
             ?: extract(&type, bufferpptr, bufferptrmax)
             ?: extract(&name, bufferpptr, bufferptrmax);
     if (status != NO_ERROR) return status;
-    mName = name;
-    if (mType == mediametrics::kTypeCString) {
-        free(u.CStringValue);
-        u.CStringValue = nullptr;
-    }
-    mType = (mediametrics::Type)type;
-    switch (mType) {
-    case mediametrics::kTypeInt32:
-        return extract(&u.int32Value, bufferpptr, bufferptrmax);
-    case mediametrics::kTypeInt64:
-        return extract(&u.int64Value, bufferpptr, bufferptrmax);
-    case mediametrics::kTypeDouble:
-        return extract(&u.doubleValue, bufferpptr, bufferptrmax);
-    case mediametrics::kTypeRate:
-        return extract(&u.rate.first, bufferpptr, bufferptrmax)
-                ?: extract(&u.rate.second, bufferpptr, bufferptrmax);
-    case mediametrics::kTypeCString:
-        status = extract(&u.CStringValue, bufferpptr, bufferptrmax);
-        if (status != NO_ERROR) mType = mediametrics::kTypeNone;
-        return status;
-    case mediametrics::kTypeNone:
-        return NO_ERROR;
+    switch (type) {
+    case mediametrics::kTypeInt32: {
+        int32_t value;
+        status = extract(&value, bufferpptr, bufferptrmax);
+        if (status != NO_ERROR) return status;
+        mElem = value;
+    } break;
+    case mediametrics::kTypeInt64: {
+        int64_t value;
+        status = extract(&value, bufferpptr, bufferptrmax);
+        if (status != NO_ERROR) return status;
+        mElem = value;
+    } break;
+    case mediametrics::kTypeDouble: {
+        double value;
+        status = extract(&value, bufferpptr, bufferptrmax);
+        if (status != NO_ERROR) return status;
+        mElem = value;
+    } break;
+    case mediametrics::kTypeRate: {
+        std::pair<int64_t, int64_t> value;
+        status = extract(&value.first, bufferpptr, bufferptrmax)
+                ?: extract(&value.second, bufferpptr, bufferptrmax);
+        if (status != NO_ERROR) return status;
+        mElem = value;
+    } break;
+    case mediametrics::kTypeCString: {
+        std::string value;
+        status = extract(&value, bufferpptr, bufferptrmax);
+        if (status != NO_ERROR) return status;
+        mElem = std::move(value);
+    } break;
+    case mediametrics::kTypeNone: {
+        mElem = std::monostate{};
+    } break;
     default:
-        mType = mediametrics::kTypeNone;
         ALOGE("%s: found bad prop type: %d, name %s",
-                __func__, mType, mName.c_str());  // no payload sent
+                __func__, (int)type, mName.c_str());  // no payload sent
         return BAD_VALUE;
     }
+    mName = name;
+    return NO_ERROR;
 }
 
 } // namespace android::mediametrics
diff --git a/media/libmediametrics/include/MediaMetricsItem.h b/media/libmediametrics/include/MediaMetricsItem.h
index f69de7a..35a0b55 100644
--- a/media/libmediametrics/include/MediaMetricsItem.h
+++ b/media/libmediametrics/include/MediaMetricsItem.h
@@ -23,7 +23,9 @@
 #include <map>
 #include <string>
 #include <sys/types.h>
+#include <variant>
 
+#include <binder/Parcel.h>
 #include <cutils/properties.h>
 #include <utils/Errors.h>
 #include <utils/KeyedVector.h>
@@ -122,39 +124,249 @@
     static void dropInstance();
     static bool submitBuffer(const char *buffer, size_t len);
 
-    static status_t writeToByteString(
-            const char *name, int32_t value, char **bufferpptr, char *bufferptrmax);
-    static status_t writeToByteString(
-            const char *name, int64_t value, char **bufferpptr, char *bufferptrmax);
-    static status_t writeToByteString(
-            const char *name, double value, char **bufferpptr, char *bufferptrmax);
-    static status_t writeToByteString(
-            const char *name, const std::pair<int64_t, int64_t> &value,
-            char **bufferpptr, char *bufferptrmax);
-    static status_t writeToByteString(
-            const char *name, char * const &value, char **bufferpptr, char *bufferptrmax);
-    static status_t writeToByteString(
-            const char *name, const char * const &value, char **bufferpptr, char *bufferptrmax);
-    struct none_t {}; // for mediametrics::kTypeNone
-    static status_t writeToByteString(
-            const char *name, const none_t &, char **bufferpptr, char *bufferptrmax);
+    template <typename T>
+    struct is_item_type {
+        static constexpr inline bool value =
+             std::is_same<T, int32_t>::value
+             || std::is_same<T, int64_t>::value
+             || std::is_same<T, double>::value
+             || std::is_same<T, std::pair<int64_t, int64_t>>:: value
+             || std::is_same<T, std::string>::value
+             || std::is_same<T, std::monostate>::value;
+    };
 
-    template<typename T>
-    static status_t sizeOfByteString(const char *name, const T& value) {
+    template <typename T>
+    struct get_type_of {
+        static_assert(is_item_type<T>::value);
+        static constexpr inline Type value =
+             std::is_same<T, int32_t>::value ? kTypeInt32
+             : std::is_same<T, int64_t>::value ? kTypeInt64
+             : std::is_same<T, double>::value ? kTypeDouble
+             : std::is_same<T, std::pair<int64_t, int64_t>>:: value ? kTypeRate
+             : std::is_same<T, std::string>::value ? kTypeCString
+             : std::is_same<T, std::monostate>::value ? kTypeNone
+         : kTypeNone;
+    };
+
+    template <typename T>
+    static size_t sizeOfByteString(const char *name, const T& value) {
+        static_assert(is_item_type<T>::value);
         return 2 + 1 + strlen(name) + 1 + sizeof(value);
     }
-    template<> // static
-    status_t sizeOfByteString(const char *name, char * const &value) {
-        return 2 + 1 + strlen(name) + 1 + strlen(value) + 1;
+    template <> // static
+    size_t sizeOfByteString(const char *name, const std::string& value) {
+        return 2 + 1 + strlen(name) + 1 + value.size() + 1;
     }
-    template<> // static
-    status_t sizeOfByteString(const char *name, const char * const &value) {
-        return 2 + 1 + strlen(name) + 1 + strlen(value) + 1;
-    }
-    template<> // static
-    status_t sizeOfByteString(const char *name, const none_t &) {
+    template <> // static
+    size_t sizeOfByteString(const char *name, const std::monostate&) {
          return 2 + 1 + strlen(name) + 1;
     }
+    // for speed
+    static size_t sizeOfByteString(const char *name, const char *value) {
+        return 2 + 1 + strlen(name) + 1 + strlen(value) + 1;
+    }
+
+    template <typename T>
+    static status_t insert(const T& val, char **bufferpptr, char *bufferptrmax) {
+        static_assert(std::is_trivially_constructible<T>::value);
+        const size_t size = sizeof(val);
+        if (*bufferpptr + size > bufferptrmax) {
+            ALOGE("%s: buffer exceeded with size %zu", __func__, size);
+            return BAD_VALUE;
+        }
+        memcpy(*bufferpptr, &val, size);
+        *bufferpptr += size;
+        return NO_ERROR;
+    }
+    template <> // static
+    status_t insert(const std::string& val, char **bufferpptr, char *bufferptrmax) {
+        const size_t size = val.size() + 1;
+        if (size > UINT16_MAX || *bufferpptr + size > bufferptrmax) {
+            ALOGE("%s: buffer exceeded with size %zu", __func__, size);
+            return BAD_VALUE;
+        }
+        memcpy(*bufferpptr, val.c_str(), size);
+        *bufferpptr += size;
+        return NO_ERROR;
+    }
+    template <> // static
+    status_t insert(const std::pair<int64_t, int64_t>& val,
+            char **bufferpptr, char *bufferptrmax) {
+        const size_t size = sizeof(val.first) + sizeof(val.second);
+        if (*bufferpptr + size > bufferptrmax) {
+            ALOGE("%s: buffer exceeded with size %zu", __func__, size);
+            return BAD_VALUE;
+        }
+        memcpy(*bufferpptr, &val.first, sizeof(val.first));
+        memcpy(*bufferpptr + sizeof(val.first), &val.second, sizeof(val.second));
+        *bufferpptr += size;
+        return NO_ERROR;
+    }
+    template <> // static
+    status_t insert(const std::monostate&, char **, char *) {
+        return NO_ERROR;
+    }
+    // for speed
+    static status_t insert(const char *val, char **bufferpptr, char *bufferptrmax) {
+        const size_t size = strlen(val) + 1;
+        if (size > UINT16_MAX || *bufferpptr + size > bufferptrmax) {
+            ALOGE("%s: buffer exceeded with size %zu", __func__, size);
+            return BAD_VALUE;
+        }
+        memcpy(*bufferpptr, val, size);
+        *bufferpptr += size;
+        return NO_ERROR;
+    }
+
+    template <typename T>
+    static status_t writeToByteString(
+            const char *name, const T& value, char **bufferpptr, char *bufferptrmax) {
+        static_assert(is_item_type<T>::value);
+        const size_t len = sizeOfByteString(name, value);
+        if (len > UINT16_MAX) return BAD_VALUE;
+        return insert((uint16_t)len, bufferpptr, bufferptrmax)
+                ?: insert((uint8_t)get_type_of<T>::value, bufferpptr, bufferptrmax)
+                ?: insert(name, bufferpptr, bufferptrmax)
+                ?: insert(value, bufferpptr, bufferptrmax);
+    }
+    // for speed
+    static status_t writeToByteString(
+            const char *name, const char *value, char **bufferpptr, char *bufferptrmax) {
+        const size_t len = sizeOfByteString(name, value);
+        if (len > UINT16_MAX) return BAD_VALUE;
+        return insert((uint16_t)len, bufferpptr, bufferptrmax)
+                ?: insert((uint8_t)kTypeCString, bufferpptr, bufferptrmax)
+                ?: insert(name, bufferpptr, bufferptrmax)
+                ?: insert(value, bufferpptr, bufferptrmax);
+    }
+
+    template <typename T>
+    static void toStringBuffer(
+            const char *name, const T& value, char *buffer, size_t length) = delete;
+    template <> // static
+    void toStringBuffer(
+            const char *name, const int32_t& value, char *buffer, size_t length) {
+        snprintf(buffer, length, "%s=%d:", name, value);
+    }
+    template <> // static
+    void toStringBuffer(
+            const char *name, const int64_t& value, char *buffer, size_t length) {
+        snprintf(buffer, length, "%s=%lld:", name, (long long)value);
+    }
+    template <> // static
+    void toStringBuffer(
+            const char *name, const double& value, char *buffer, size_t length) {
+        snprintf(buffer, length, "%s=%e:", name, value);
+    }
+    template <> // static
+    void toStringBuffer(
+            const char *name, const std::pair<int64_t, int64_t>& value,
+            char *buffer, size_t length) {
+        snprintf(buffer, length, "%s=%lld/%lld:",
+                name, (long long)value.first, (long long)value.second);
+    }
+    template <> // static
+    void toStringBuffer(
+            const char *name, const std::string& value, char *buffer, size_t length) {
+        // TODO sanitize string for ':' '='
+        snprintf(buffer, length, "%s=%s:", name, value.c_str());
+    }
+    template <> // static
+    void toStringBuffer(
+            const char *name, const std::monostate&, char *buffer, size_t length) {
+        snprintf(buffer, length, "%s=():", name);
+    }
+
+    template <typename T>
+    static status_t writeToParcel(
+            const char *name, const T& value, Parcel *parcel) = delete;
+    template <> // static
+    status_t writeToParcel(
+            const char *name, const int32_t& value, Parcel *parcel) {
+        return parcel->writeCString(name)
+               ?: parcel->writeInt32(get_type_of<int32_t>::value)
+               ?: parcel->writeInt32(value);
+    }
+    template <> // static
+    status_t writeToParcel(
+            const char *name, const int64_t& value, Parcel *parcel) {
+        return parcel->writeCString(name)
+               ?: parcel->writeInt32(get_type_of<int64_t>::value)
+               ?: parcel->writeInt64(value);
+    }
+    template <> // static
+    status_t writeToParcel(
+            const char *name, const double& value, Parcel *parcel) {
+        return parcel->writeCString(name)
+               ?: parcel->writeInt32(get_type_of<double>::value)
+               ?: parcel->writeDouble(value);
+    }
+    template <> // static
+    status_t writeToParcel(
+            const char *name, const std::pair<int64_t, int64_t>& value, Parcel *parcel) {
+        return parcel->writeCString(name)
+               ?: parcel->writeInt32(get_type_of< std::pair<int64_t, int64_t>>::value)
+               ?: parcel->writeInt64(value.first)
+               ?: parcel->writeInt64(value.second);
+    }
+    template <> // static
+    status_t writeToParcel(
+            const char *name, const std::string& value, Parcel *parcel) {
+        return parcel->writeCString(name)
+               ?: parcel->writeInt32(get_type_of<std::string>::value)
+               ?: parcel->writeCString(value.c_str());
+    }
+    template <> // static
+    status_t writeToParcel(
+            const char *name, const std::monostate&, Parcel *parcel) {
+        return parcel->writeCString(name)
+               ?: parcel->writeInt32(get_type_of<std::monostate>::value);
+    }
+
+    template <typename T>
+    static status_t extract(T *val, const char **bufferpptr, const char *bufferptrmax) {
+        static_assert(std::is_trivially_constructible<T>::value);
+        const size_t size = sizeof(*val);
+        if (*bufferpptr + size > bufferptrmax) {
+            ALOGE("%s: buffer exceeded with size %zu", __func__, size);
+            return BAD_VALUE;
+        }
+        memcpy(val, *bufferpptr, size);
+        *bufferpptr += size;
+        return NO_ERROR;
+    }
+    template <> // static
+    status_t extract(std::string *val, const char **bufferpptr, const char *bufferptrmax) {
+        const char *ptr = *bufferpptr;
+        while (*ptr != 0) {
+            if (ptr >= bufferptrmax) {
+                ALOGE("%s: buffer exceeded", __func__);
+                return BAD_VALUE;
+            }
+            ++ptr;
+        }
+        const size_t size = (ptr - *bufferpptr) + 1;
+        *val = *bufferpptr;
+        *bufferpptr += size;
+        return NO_ERROR;
+    }
+    template <> // static
+    status_t extract(std::pair<int64_t, int64_t> *val,
+            const char **bufferpptr, const char *bufferptrmax) {
+        const size_t size = sizeof(val->first) + sizeof(val->second);
+        if (*bufferpptr + size > bufferptrmax) {
+            ALOGE("%s: buffer exceeded with size %zu", __func__, size);
+            return BAD_VALUE;
+        }
+        memcpy(&val->first, *bufferpptr, sizeof(val->first));
+        memcpy(&val->second, *bufferpptr + sizeof(val->first), sizeof(val->second));
+        *bufferpptr += size;
+        return NO_ERROR;
+    }
+    template <> // static
+    status_t extract(std::monostate *, const char **, const char *) {
+        return NO_ERROR;
+    }
 };
 
 /**
@@ -514,9 +726,9 @@
     }
     // Caller owns the returned string
     bool getCString(const char *key, char **value) const {
-        const char *cs;
-        if (get(key, &cs)) {
-            *value = cs != nullptr ? strdup(cs) : nullptr;
+        std::string s;
+        if (get(key, &s)) {
+            *value = strdup(s.c_str());
             return true;
         }
         return false;
@@ -575,47 +787,26 @@
     int32_t writeToParcel0(Parcel *) const;
     int32_t readFromParcel0(const Parcel&);
 
-    // checks equality even with nullptr.
-    static bool stringEquals(const char *a, const char *b) {
-        if (a == nullptr) {
-            return b == nullptr;
-        } else {
-            return b != nullptr && strcmp(a, b) == 0;
-        }
-    }
-
 public:
+
     class Prop {
     public:
+        using Elem = std::variant<
+                std::monostate,               // kTypeNone
+                int32_t,                      // kTypeInt32
+                int64_t,                      // kTypeInt64
+                double,                       // kTypeDouble
+                std::string,                  // kTypeCString
+                std::pair<int64_t, int64_t>   // kTypeRate
+                >;
+
         Prop() = default;
         Prop(const Prop& other) {
            *this = other;
         }
         Prop& operator=(const Prop& other) {
             mName = other.mName;
-            mType = other.mType;
-            switch (mType) {
-            case mediametrics::kTypeInt32:
-                u.int32Value = other.u.int32Value;
-                break;
-            case mediametrics::kTypeInt64:
-                u.int64Value = other.u.int64Value;
-                break;
-            case mediametrics::kTypeDouble:
-                u.doubleValue = other.u.doubleValue;
-                break;
-            case mediametrics::kTypeCString:
-                u.CStringValue = strdup(other.u.CStringValue);
-                break;
-            case mediametrics::kTypeRate:
-                u.rate = other.u.rate;
-                break;
-            case mediametrics::kTypeNone:
-                break;
-            default:
-                // abort?
-                break;
-            }
+            mElem = other.mElem;
             return *this;
         }
         Prop(Prop&& other) {
@@ -623,51 +814,12 @@
         }
         Prop& operator=(Prop&& other) {
             mName = std::move(other.mName);
-            mType = other.mType;
-            switch (mType) {
-            case mediametrics::kTypeInt32:
-                u.int32Value = other.u.int32Value;
-                break;
-            case mediametrics::kTypeInt64:
-                u.int64Value = other.u.int64Value;
-                break;
-            case mediametrics::kTypeDouble:
-                u.doubleValue = other.u.doubleValue;
-                break;
-            case mediametrics::kTypeCString:
-                u.CStringValue = other.u.CStringValue;
-                break;
-            case mediametrics::kTypeRate:
-                u.rate = other.u.rate;
-                break;
-            case mediametrics::kTypeNone:
-                break;
-            default:
-                // abort?
-                break;
-            }
-            other.mType = mediametrics::kTypeNone;
+            mElem = std::move(other.mElem);
             return *this;
         }
 
         bool operator==(const Prop& other) const {
-            if (mName != other.mName
-                    || mType != other.mType) return false;
-            switch (mType) {
-            case mediametrics::kTypeInt32:
-                return u.int32Value == other.u.int32Value;
-            case mediametrics::kTypeInt64:
-                return u.int64Value == other.u.int64Value;
-            case mediametrics::kTypeDouble:
-                return u.doubleValue == other.u.doubleValue;
-            case mediametrics::kTypeCString:
-                return stringEquals(u.CStringValue, other.u.CStringValue);
-            case mediametrics::kTypeRate:
-                return u.rate == other.u.rate;
-            case mediametrics::kTypeNone:
-            default:
-                return true;
-            }
+            return mName == other.mName && mElem == other.mElem;
         }
         bool operator!=(const Prop& other) const {
             return !(*this == other);
@@ -675,18 +827,10 @@
 
         void clear() {
             mName.clear();
-            clearValue();
+            mElem = std::monostate{};
         }
         void clearValue() {
-            if (mType == mediametrics::kTypeCString) {
-                free(u.CStringValue);
-                u.CStringValue = nullptr;
-            }
-            mType = mediametrics::kTypeNone;
-        }
-
-        mediametrics::Type getType() const {
-            return mType;
+            mElem = std::monostate{};
         }
 
         const char *getName() const {
@@ -695,8 +839,7 @@
 
         void swap(Prop& other) {
             std::swap(mName, other.mName);
-            std::swap(mType, other.mType);
-            std::swap(u, other.u);
+            std::swap(mElem, other.mElem);
         }
 
         void setName(const char *name) {
@@ -708,176 +851,69 @@
         }
 
         template <typename T> void visit(T f) const {
-            switch (mType) {
-            case mediametrics::kTypeInt32:
-                f(u.int32Value);
-                return;
-            case mediametrics::kTypeInt64:
-                f(u.int64Value);
-                return;
-            case mediametrics::kTypeDouble:
-                f(u.doubleValue);
-                return;
-            case mediametrics::kTypeRate:
-                f(u.rate);
-                return;
-            case mediametrics::kTypeCString:
-                f(u.CStringValue);
-                return;
-            default:
-                return;
+            std::visit(f, mElem);
+        }
+
+        template <typename T> bool get(T *value) const {
+            auto pval = std::get_if<T>(&mElem);
+            if (pval != nullptr) {
+                *value = *pval;
+                return true;
+            }
+            return false;
+        }
+
+        template <typename T> void set(const T& value) {
+            mElem = value;
+        }
+
+        template <typename T> void add(const T& value) {
+            auto pval = std::get_if<T>(&mElem);
+            if (pval != nullptr) {
+                *pval += value;
+            } else {
+                mElem = value;
             }
         }
 
-        template <typename T> bool get(T *value) const = delete;
-        template <>
-        bool get(int32_t *value) const {
-           if (mType != mediametrics::kTypeInt32) return false;
-           if (value != nullptr) *value = u.int32Value;
-           return true;
-        }
-        template <>
-        bool get(int64_t *value) const {
-           if (mType != mediametrics::kTypeInt64) return false;
-           if (value != nullptr) *value = u.int64Value;
-           return true;
-        }
-        template <>
-        bool get(double *value) const {
-           if (mType != mediametrics::kTypeDouble) return false;
-           if (value != nullptr) *value = u.doubleValue;
-           return true;
-        }
-        template <>
-        bool get(const char** value) const {
-            if (mType != mediametrics::kTypeCString) return false;
-            if (value != nullptr) *value = u.CStringValue;
-            return true;
-        }
-        template <>
-        bool get(std::string* value) const {
-            if (mType != mediametrics::kTypeCString) return false;
-            if (value != nullptr) *value = u.CStringValue;
-            return true;
-        }
-        template <>
-        bool get(std::pair<int64_t, int64_t> *value) const {
-           if (mType != mediametrics::kTypeRate) return false;
-           if (value != nullptr) {
-               *value = u.rate;
-           }
-           return true;
-        }
-
-        template <typename T> void set(const T& value) = delete;
-        template <>
-        void set(const int32_t& value) {
-            mType = mediametrics::kTypeInt32;
-            u.int32Value = value;
-        }
-        template <>
-        void set(const int64_t& value) {
-            mType = mediametrics::kTypeInt64;
-            u.int64Value = value;
-        }
-        template <>
-        void set(const double& value) {
-            mType = mediametrics::kTypeDouble;
-            u.doubleValue = value;
-        }
-        template <>
-        void set(const char* const& value) {
-            if (mType == mediametrics::kTypeCString) {
-                free(u.CStringValue);
+        template <> void add(const std::pair<int64_t, int64_t>& value) {
+            auto pval = std::get_if<std::pair<int64_t, int64_t>>(&mElem);
+            if (pval != nullptr) {
+                pval->first += value.first;
+                pval->second += value.second;
             } else {
-                mType = mediametrics::kTypeCString;
-            }
-            if (value == nullptr) {
-                u.CStringValue = nullptr;
-            } else {
-                size_t len = strlen(value);
-                if (len > UINT16_MAX - 1) {
-                    len = UINT16_MAX - 1;
-                }
-                u.CStringValue = (char *)malloc(len + 1);
-                strncpy(u.CStringValue, value, len);
-                u.CStringValue[len] = 0;
-            }
-        }
-        template <>
-        void set(const std::pair<int64_t, int64_t> &value) {
-            mType = mediametrics::kTypeRate;
-            u.rate = {value.first, value.second};
-        }
-
-        template <typename T> void add(const T& value) = delete;
-        template <>
-        void add(const int32_t& value) {
-            if (mType == mediametrics::kTypeInt32) {
-                u.int32Value += value;
-            } else {
-                mType = mediametrics::kTypeInt32;
-                u.int32Value = value;
-            }
-        }
-        template <>
-        void add(const int64_t& value) {
-            if (mType == mediametrics::kTypeInt64) {
-                u.int64Value += value;
-            } else {
-                mType = mediametrics::kTypeInt64;
-                u.int64Value = value;
-            }
-        }
-        template <>
-        void add(const double& value) {
-            if (mType == mediametrics::kTypeDouble) {
-                u.doubleValue += value;
-            } else {
-                mType = mediametrics::kTypeDouble;
-                u.doubleValue = value;
-            }
-        }
-        template <>
-        void add(const std::pair<int64_t, int64_t>& value) {
-            if (mType == mediametrics::kTypeRate) {
-                u.rate.first += value.first;
-                u.rate.second += value.second;
-            } else {
-                mType = mediametrics::kTypeRate;
-                u.rate = value;
+                mElem = value;
             }
         }
 
-        status_t writeToParcel(Parcel *data) const;
+        status_t writeToParcel(Parcel *parcel) const {
+            return std::visit([this, parcel](auto &value) {
+                    return BaseItem::writeToParcel(mName.c_str(), value, parcel);}, mElem);
+        }
+
+        void toStringBuffer(char *buffer, size_t length) const {
+            return std::visit([this, buffer, length](auto &value) {
+                BaseItem::toStringBuffer(mName.c_str(), value, buffer, length);}, mElem);
+        }
+
+        size_t getByteStringSize() const {
+            return std::visit([this](auto &value) {
+                return BaseItem::sizeOfByteString(mName.c_str(), value);}, mElem);
+        }
+
+        status_t writeToByteString(char **bufferpptr, char *bufferptrmax) const {
+            return std::visit([this, bufferpptr, bufferptrmax](auto &value) {
+                return BaseItem::writeToByteString(mName.c_str(), value, bufferpptr, bufferptrmax);
+            }, mElem);
+        }
+
         status_t readFromParcel(const Parcel& data);
-        void toString(char *buffer, size_t length) const;
-        size_t getByteStringSize() const;
-        status_t writeToByteString(char **bufferpptr, char *bufferptrmax) const;
+
         status_t readFromByteString(const char **bufferpptr, const char *bufferptrmax);
 
-    // TODO: consider converting to std::variant
     private:
         std::string mName;
-        mediametrics::Type mType = mediametrics::kTypeNone;
-        union u__ {
-            u__() { zero(); }
-            u__(u__ &&other) {
-                *this = std::move(other);
-            }
-            u__& operator=(u__ &&other) {
-                memcpy(this, &other, sizeof(*this));
-                other.zero();
-                return *this;
-            }
-            void zero() { memset(this, 0, sizeof(*this)); }
-
-            int32_t int32Value;
-            int64_t int64Value;
-            double doubleValue;
-            char *CStringValue;
-            std::pair<int64_t, int64_t> rate;
-        } u;
+        Elem mElem;
     };
 
     // Iteration of props within item
diff --git a/services/mediametrics/tests/mediametrics_tests.cpp b/services/mediametrics/tests/mediametrics_tests.cpp
index 285a1ba..55ce82b 100644
--- a/services/mediametrics/tests/mediametrics_tests.cpp
+++ b/services/mediametrics/tests/mediametrics_tests.cpp
@@ -274,9 +274,9 @@
           ASSERT_EQ(3.125, d);
           mask |= 4;
       } else if (!strcmp(name, "string")) {
-          const char *s;
+          std::string s;
           ASSERT_TRUE(prop.get(&s));
-          ASSERT_EQ(0, strcmp(s, "abc"));
+          ASSERT_EQ("abc", s);
           mask |= 8;
       } else if (!strcmp(name, "rate")) {
           std::pair<int64_t, int64_t> r;
@@ -323,9 +323,9 @@
           ASSERT_EQ(3.125, d);
           mask |= 4;
       } else if (!strcmp(name, "string")) {
-          const char *s;
+          std::string s;
           ASSERT_TRUE(prop.get(&s));
-          ASSERT_EQ(0, strcmp(s, "abcdefghijklmnopqrstuvwxyz"));
+          ASSERT_EQ("abcdefghijklmnopqrstuvwxyz", s);
           mask |= 8;
       } else if (!strcmp(name, "rate")) {
           std::pair<int64_t, int64_t> r;