audio: Use strings for EffectDescriptor.{name|implementor}
The use of fixed size char arrays was a direct copy
of the C structure approach. In HIDL, the canonical approach
is to use strings.
Bug: 181883090
Test: atest android.hardware.audio.effect@7.0-util_tests
Change-Id: I468810e07e5ba8b3bd6f03c5acfa24009ef2e48a
diff --git a/audio/effect/7.0/types.hal b/audio/effect/7.0/types.hal
index bb2d7b3..8f4f885 100644
--- a/audio/effect/7.0/types.hal
+++ b/audio/effect/7.0/types.hal
@@ -220,9 +220,9 @@
*/
uint16_t memoryUsage;
/** Human readable effect name. */
- uint8_t[64] name;
+ string name;
/** Human readable effect implementor name. */
- uint8_t[64] implementor;
+ string implementor;
};
/**
diff --git a/audio/effect/all-versions/default/util/EffectUtils.cpp b/audio/effect/all-versions/default/util/EffectUtils.cpp
index 1c0419a..b4382dc 100644
--- a/audio/effect/all-versions/default/util/EffectUtils.cpp
+++ b/audio/effect/all-versions/default/util/EffectUtils.cpp
@@ -16,12 +16,17 @@
#include <memory.h>
+#define LOG_TAG "EffectUtils"
+#include <log/log.h>
+
#include <HidlUtils.h>
#include <UuidUtils.h>
#include <common/all-versions/VersionUtils.h>
#include "util/EffectUtils.h"
+#define ARRAY_SIZE(a) (sizeof(a) / sizeof(*(a)))
+
using ::android::hardware::audio::common::CPP_VERSION::implementation::HidlUtils;
using ::android::hardware::audio::common::CPP_VERSION::implementation::UuidUtils;
using ::android::hardware::audio::common::utils::EnumBitfield;
@@ -156,23 +161,52 @@
descriptor->flags = EnumBitfield<EffectFlags>(halDescriptor.flags);
descriptor->cpuLoad = halDescriptor.cpuLoad;
descriptor->memoryUsage = halDescriptor.memoryUsage;
+#if MAJOR_VERSION <= 6
memcpy(descriptor->name.data(), halDescriptor.name, descriptor->name.size());
memcpy(descriptor->implementor.data(), halDescriptor.implementor,
descriptor->implementor.size());
+#else
+ descriptor->name = hidl_string(halDescriptor.name, ARRAY_SIZE(halDescriptor.name));
+ descriptor->implementor =
+ hidl_string(halDescriptor.implementor, ARRAY_SIZE(halDescriptor.implementor));
+#endif
return NO_ERROR;
}
status_t EffectUtils::effectDescriptorToHal(const EffectDescriptor& descriptor,
effect_descriptor_t* halDescriptor) {
+ status_t result = NO_ERROR;
UuidUtils::uuidToHal(descriptor.type, &halDescriptor->type);
UuidUtils::uuidToHal(descriptor.uuid, &halDescriptor->uuid);
halDescriptor->flags = static_cast<uint32_t>(descriptor.flags);
halDescriptor->cpuLoad = descriptor.cpuLoad;
halDescriptor->memoryUsage = descriptor.memoryUsage;
+#if MAJOR_VERSION <= 6
memcpy(halDescriptor->name, descriptor.name.data(), descriptor.name.size());
memcpy(halDescriptor->implementor, descriptor.implementor.data(),
descriptor.implementor.size());
- return NO_ERROR;
+#else
+ // According to 'dumpEffectDescriptor' 'name' and 'implementor' must be NUL-terminated.
+ size_t nameSize = descriptor.name.size();
+ if (nameSize >= ARRAY_SIZE(halDescriptor->name)) {
+ ALOGE("effect name is too long: %zu (%zu max)", nameSize,
+ ARRAY_SIZE(halDescriptor->name) - 1);
+ nameSize = ARRAY_SIZE(halDescriptor->name) - 1;
+ result = BAD_VALUE;
+ }
+ strncpy(halDescriptor->name, descriptor.name.c_str(), nameSize);
+ halDescriptor->name[nameSize] = '\0';
+ size_t implementorSize = descriptor.implementor.size();
+ if (implementorSize >= ARRAY_SIZE(halDescriptor->implementor)) {
+ ALOGE("effect implementor is too long: %zu (%zu max)", implementorSize,
+ ARRAY_SIZE(halDescriptor->implementor) - 1);
+ implementorSize = ARRAY_SIZE(halDescriptor->implementor) - 1;
+ result = BAD_VALUE;
+ }
+ strncpy(halDescriptor->implementor, descriptor.implementor.c_str(), implementorSize);
+ halDescriptor->implementor[implementorSize] = '\0';
+#endif
+ return result;
}
} // namespace implementation
diff --git a/audio/effect/all-versions/default/util/tests/effectutils_tests.cpp b/audio/effect/all-versions/default/util/tests/effectutils_tests.cpp
index 7eb8cd2..f3651de 100644
--- a/audio/effect/all-versions/default/util/tests/effectutils_tests.cpp
+++ b/audio/effect/all-versions/default/util/tests/effectutils_tests.cpp
@@ -134,8 +134,20 @@
EXPECT_EQ(format, formatBackIn);
}
+TEST(EffectUtils, ConvertInvalidDescriptor) {
+ effect_descriptor_t halDesc;
+ EffectDescriptor longName{};
+ longName.name = std::string(EFFECT_STRING_LEN_MAX, 'x');
+ EXPECT_EQ(BAD_VALUE, EffectUtils::effectDescriptorToHal(longName, &halDesc));
+ EffectDescriptor longImplementor{};
+ longImplementor.implementor = std::string(EFFECT_STRING_LEN_MAX, 'x');
+ EXPECT_EQ(BAD_VALUE, EffectUtils::effectDescriptorToHal(longImplementor, &halDesc));
+}
+
TEST(EffectUtils, ConvertDescriptor) {
EffectDescriptor desc{};
+ desc.name = "test";
+ desc.implementor = "foo";
effect_descriptor_t halDesc;
EXPECT_EQ(NO_ERROR, EffectUtils::effectDescriptorToHal(desc, &halDesc));
EffectDescriptor descBack;