From 0c52b65211fc296a3139bcfb094ff2e22371baac Mon Sep 17 00:00:00 2001 From: George Burgess IV Date: Mon, 24 Feb 2020 11:37:35 -0800 Subject: [PATCH 1/2] use vector::erase with std::remove_if std::remove_if will only move elements around within the range it was handed. In order to have the elements actually removed from the vector, we need to erase() them. Caught by clang-tidy: /buildbot/src/googleplex-android/rvc-release/frameworks/av/media/codec2/vndk/util/C2InterfaceUtils.cpp:219:9: warning: the value returned by this function should be used [bugprone-unused-return-value] (and 3 others) Bug: 154665437 Test: TreeHugger Change-Id: I9244baf7d6f46cce4c2d8f66fcf57b19c19d32ab Merged-In: I9244baf7d6f46cce4c2d8f66fcf57b19c19d32ab --- media/codec2/vndk/util/C2InterfaceUtils.cpp | 32 +++++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/media/codec2/vndk/util/C2InterfaceUtils.cpp b/media/codec2/vndk/util/C2InterfaceUtils.cpp index 61ec9115be..0c1729b69a 100644 --- a/media/codec2/vndk/util/C2InterfaceUtils.cpp +++ b/media/codec2/vndk/util/C2InterfaceUtils.cpp @@ -216,9 +216,14 @@ C2SupportedFlags C2SupportedFlags::limitedTo(const C2SupportedFlags &li if (limit.contains(minMask) && contains(minMask)) { values[0] = minMask; // keep only flags that are covered by limit - std::remove_if(values.begin(), values.end(), [&limit, minMask](const C2Value::Primitive &v) -> bool { - T value = v.ref() | minMask; - return value == minMask || !limit.contains(value); }); + values.erase(std::remove_if(values.begin(), values.end(), + [&limit, minMask]( + const C2Value::Primitive &v) -> bool { + T value = v.ref() | minMask; + return value == minMask || + !limit.contains(value); + }), + values.end()); // we also need to do it vice versa for (const C2Value::Primitive &v : _mValues) { T value = v.ref() | minMask; @@ -264,24 +269,33 @@ bool C2SupportedValueSet::contains(T value) const { template C2SupportedValueSet C2SupportedValueSet::limitedTo(const C2SupportedValueSet &limit) const { std::vector values = _mValues; // make a copy - std::remove_if(values.begin(), values.end(), [&limit](const C2Value::Primitive &v) -> bool { - return !limit.contains(v.ref()); }); + values.erase(std::remove_if(values.begin(), values.end(), + [&limit](const C2Value::Primitive &v) -> bool { + return !limit.contains(v.ref()); + }), + values.end()); return C2SupportedValueSet(std::move(values)); } template C2SupportedValueSet C2SupportedValueSet::limitedTo(const C2SupportedRange &limit) const { std::vector values = _mValues; // make a copy - std::remove_if(values.begin(), values.end(), [&limit](const C2Value::Primitive &v) -> bool { - return !limit.contains(v.ref()); }); + values.erase(std::remove_if(values.begin(), values.end(), + [&limit](const C2Value::Primitive &v) -> bool { + return !limit.contains(v.ref()); + }), + values.end()); return C2SupportedValueSet(std::move(values)); } template C2SupportedValueSet C2SupportedValueSet::limitedTo(const C2SupportedFlags &limit) const { std::vector values = _mValues; // make a copy - std::remove_if(values.begin(), values.end(), [&limit](const C2Value::Primitive &v) -> bool { - return !limit.contains(v.ref()); }); + values.erase(std::remove_if(values.begin(), values.end(), + [&limit](const C2Value::Primitive &v) -> bool { + return !limit.contains(v.ref()); + }), + values.end()); return C2SupportedValueSet(std::move(values)); } From c4b71c9302c61ed47129991c4608034d59f05a29 Mon Sep 17 00:00:00 2001 From: Saketh Sathuvalli Date: Wed, 11 Mar 2020 13:06:27 +0530 Subject: [PATCH 2/2] Fix for potential NULL de-reference issue in Reverb Fix for potential NULL de-reference issue in EffectReverb.cpp file Test: android.media.cts.AudioEffectTest Test: android.media.cts.AudioPreProcessingTest Test: android.media.cts.BassBoostTest Test: android.media.cts.EnvReverbTest Test: android.media.cts.EqualizerTest Test: android.media.cts.LoudnessEnhancerTest Test: android.media.cts.PresetReverbTest Test: android.media.cts.VirtualizerTest Test: android.media.cts.VisualizerTest Bug: 77731062, 154665188 Change-Id: Ib293173bb2512553bacb9a35dcf0f82bb290ede8 Merged-In: Ib293173bb2512553bacb9a35dcf0f82bb290ede8 --- media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp b/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp index 1cb81a66fa..39f5bb6708 100644 --- a/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp +++ b/media/libeffects/lvm/wrapper/Reverb/EffectReverb.cpp @@ -1906,11 +1906,15 @@ int Reverb_command(effect_handle_t self, //ALOGV("\tReverb_command cmdCode Case: " // "EFFECT_CMD_GET_PARAM start"); effect_param_t *p = (effect_param_t *)pCmdData; + if (pCmdData == nullptr) { + ALOGW("\tLVM_ERROR : pCmdData is NULL"); + return -EINVAL; + } if (SIZE_MAX - sizeof(effect_param_t) < (size_t)p->psize) { android_errorWriteLog(0x534e4554, "26347509"); return -EINVAL; } - if (pCmdData == NULL || cmdSize < sizeof(effect_param_t) || + if (cmdSize < sizeof(effect_param_t) || cmdSize < (sizeof(effect_param_t) + p->psize) || pReplyData == NULL || replySize == NULL || *replySize < (sizeof(effect_param_t) + p->psize)) {