Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8fca3002 authored by Mikhail Naganov's avatar Mikhail Naganov
Browse files

audio: Fix fixed size char array conversions 

For legacy HAL strings that are fixed size arrays
the conversion code was using the array size for
the HIDL string size. This lead to logging of error
messages during reverse conversion.

Fixed issue and refactored code to avoid duplication.

Bug: 181269159
Test: atest android.hardware.audio.effect@7.0-util_tests
      also, verify that no error messages from EffectUtil
      appear during boot and audio playback
Change-Id: Iac36ff33e65c502966ac2b7a4870cb5830545b23
parent 6a022ad1

audio/effect/all-versions/default/util/EffectUtils.cpp

+30 −24
Original line number Diff line number Diff line
@@ -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 <std::size_t N>

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 <std::size_t N>

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;

}

audio/effect/all-versions/default/util/tests/effectutils_tests.cpp

+17 −0
Original line number Diff line number Diff line
@@ -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<char>(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));

    }

}