diff --git a/audio/effect/all-versions/default/util/EffectUtils.cpp b/audio/effect/all-versions/default/util/EffectUtils.cpp index b4382dc2d6..1156d211a1 100644 --- a/audio/effect/all-versions/default/util/EffectUtils.cpp +++ b/audio/effect/all-versions/default/util/EffectUtils.cpp @@ -25,8 +25,6 @@ #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; @@ -154,6 +152,29 @@ status_t EffectUtils::effectConfigToHal(const EffectConfig& config, effect_confi return result; } +template +inline hidl_string charBufferFromHal(const char (&halBuf)[N]) { + // Even if the original field contains a non-terminated string, hidl_string + // adds a NUL terminator. + return hidl_string(halBuf, strnlen(halBuf, N)); +} + +template +inline status_t charBufferToHal(const hidl_string& str, char (&halBuf)[N], const char* fieldName) { + static_assert(N > 0); + const size_t halBufChars = N - 1; // Reserve one character for terminating NUL. + status_t result = NO_ERROR; + size_t strSize = str.size(); + if (strSize > halBufChars) { + ALOGE("%s is too long: %zu (%zu max)", fieldName, strSize, halBufChars); + strSize = halBufChars; + result = BAD_VALUE; + } + strncpy(halBuf, str.c_str(), strSize); + halBuf[strSize] = '\0'; + return result; +} + status_t EffectUtils::effectDescriptorFromHal(const effect_descriptor_t& halDescriptor, EffectDescriptor* descriptor) { UuidUtils::uuidFromHal(halDescriptor.type, &descriptor->type); @@ -166,9 +187,8 @@ status_t EffectUtils::effectDescriptorFromHal(const effect_descriptor_t& halDesc 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)); + descriptor->name = charBufferFromHal(halDescriptor.name); + descriptor->implementor = charBufferFromHal(halDescriptor.implementor); #endif return NO_ERROR; } @@ -186,25 +206,11 @@ status_t EffectUtils::effectDescriptorToHal(const EffectDescriptor& descriptor, memcpy(halDescriptor->implementor, descriptor.implementor.data(), descriptor.implementor.size()); #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'; + // According to 'dumpEffectDescriptor', 'name' and 'implementor' must be NUL-terminated. + CONVERT_CHECKED(charBufferToHal(descriptor.name, halDescriptor->name, "effect name"), result); + CONVERT_CHECKED(charBufferToHal(descriptor.implementor, halDescriptor->implementor, + "effect implementor"), + result); #endif return result; } 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 f3651de236..d021fa0b92 100644 --- a/audio/effect/all-versions/default/util/tests/effectutils_tests.cpp +++ b/audio/effect/all-versions/default/util/tests/effectutils_tests.cpp @@ -154,3 +154,20 @@ TEST(EffectUtils, ConvertDescriptor) { EXPECT_EQ(NO_ERROR, EffectUtils::effectDescriptorFromHal(halDesc, &descBack)); EXPECT_EQ(desc, descBack); } + +TEST(EffectUtils, ConvertNameAndImplementor) { + for (size_t i = 0; i < EFFECT_STRING_LEN_MAX; ++i) { + effect_descriptor_t halDesc{}; + for (size_t c = 0; c < i; ++c) { // '<' to accommodate NUL terminator. + halDesc.name[c] = halDesc.implementor[c] = 'A' + static_cast(c); + } + EffectDescriptor desc; + EXPECT_EQ(NO_ERROR, EffectUtils::effectDescriptorFromHal(halDesc, &desc)); + effect_descriptor_t halDescBack; + EXPECT_EQ(NO_ERROR, EffectUtils::effectDescriptorToHal(desc, &halDescBack)); + EXPECT_EQ(i, strlen(halDescBack.name)); + EXPECT_EQ(i, strlen(halDescBack.implementor)); + EXPECT_EQ(0, strcmp(halDesc.name, halDescBack.name)); + EXPECT_EQ(0, strcmp(halDesc.implementor, halDescBack.implementor)); + } +}