Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libandroidfw: Revert null check in ApplyStyle

Browse Source 
The out parameter `out_indices` is expected to be non-null
and so the extra null check adds a cost to performance
and opens the door to misusing the API by not supplying
`out_indices`.

Test: make libandroidfw_tests
Change-Id: Ie66fd837c5e24ec2838156e7b67f54c15cd27933
This commit is contained in:
Adam Lesinski
2017-01-05 17:03:55 -08:00
parent 0a454c39cb
commit 06d3e8fec7
3 changed files with 29 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
libs/androidfw/AttributeResolution.cpp
View File

@@ -418,8 +418,10 @@ void ApplyStyle(ResTable::Theme* theme, ResXMLParser* xml_parser, uint32_t def_s
out_values[STYLE_CHANGING_CONFIGURATIONS] = type_set_flags;
out_values[STYLE_DENSITY] = config.density;
if (out_indices != nullptr && value.dataType != Res_value::TYPE_NULL) {
if (value.dataType != Res_value::TYPE_NULL) {
indices_idx++;
// out_indices must NOT be nullptr.
out_indices[indices_idx] = ii;
}
@@ -428,9 +430,8 @@ void ApplyStyle(ResTable::Theme* theme, ResXMLParser* xml_parser, uint32_t def_s
res.unlock();
if (out_indices != nullptr) {
out_indices[0] = indices_idx;
}
// out_indices must NOT be nullptr.
out_indices[0] = indices_idx;
}
bool RetrieveAttributes(const ResTable* res, ResXMLParser* xml_parser,

6
libs/androidfw/include/androidfw/AttributeResolution.h
View File

@@ -40,14 +40,20 @@ enum {
// TODO(adamlesinski): Run performance tests against these methods and a new, single method
// that uses all the sources and branches to the right ones within the inner loop.
// `out_values` must NOT be nullptr.
// `out_indices` may be nullptr.
bool ResolveAttrs(ResTable::Theme* theme, uint32_t def_style_attr, uint32_t def_style_res,
uint32_t* src_values, size_t src_values_length, uint32_t* attrs,
size_t attrs_length, uint32_t* out_values, uint32_t* out_indices);
// `out_values` must NOT be nullptr.
// `out_indices` is NOT optional and must NOT be nullptr.
void ApplyStyle(ResTable::Theme* theme, ResXMLParser* xml_parser, uint32_t def_style_attr,
uint32_t def_style_res, const uint32_t* attrs, size_t attrs_length,
uint32_t* out_values, uint32_t* out_indices);
// `out_values` must NOT be nullptr.
// `out_indices` may be nullptr.
bool RetrieveAttributes(const ResTable* res, ResXMLParser* xml_parser, uint32_t* attrs,
size_t attrs_length, uint32_t* out_values, uint32_t* out_indices);

38
libs/androidfw/tests/AttributeResolution_test.cpp
View File

@@ -16,6 +16,8 @@
#include "androidfw/AttributeResolution.h"
#include <array>
#include "android-base/file.h"
#include "android-base/logging.h"
#include "android-base/macros.h"
@@ -67,15 +69,13 @@ TEST_F(AttributeResolutionTest, Theme) {
ResTable::Theme theme(table_);
ASSERT_EQ(NO_ERROR, theme.applyStyle(R::style::StyleTwo));
uint32_t attrs[] = {R::attr::attr_one, R::attr::attr_two, R::attr::attr_three,
R::attr::attr_four};
std::vector<uint32_t> values;
values.resize(arraysize(attrs) * 6);
std::array<uint32_t, 4> attrs{
{R::attr::attr_one, R::attr::attr_two, R::attr::attr_three, R::attr::attr_four}};
std::array<uint32_t, attrs.size() * STYLE_NUM_ENTRIES> values;
ASSERT_TRUE(ResolveAttrs(&theme, 0 /*def_style_attr*/, 0 /*def_style_res*/,
nullptr /*src_values*/, 0 /*src_values_length*/,
attrs, arraysize(attrs), values.data(),
nullptr /*out_indices*/));
nullptr /*src_values*/, 0 /*src_values_length*/, attrs.data(),
attrs.size(), values.data(), nullptr /*out_indices*/));
const uint32_t public_flag = ResTable_typeSpec::SPEC_PUBLIC;
@@ -112,13 +112,12 @@ TEST_F(AttributeResolutionTest, Theme) {
}
TEST_F(AttributeResolutionXmlTest, XmlParser) {
uint32_t attrs[] = {R::attr::attr_one, R::attr::attr_two, R::attr::attr_three,
R::attr::attr_four};
std::vector<uint32_t> values;
values.resize(arraysize(attrs) * 6);
std::array<uint32_t, 4> attrs{
{R::attr::attr_one, R::attr::attr_two, R::attr::attr_three, R::attr::attr_four}};
std::array<uint32_t, attrs.size() * STYLE_NUM_ENTRIES> values;
ASSERT_TRUE(RetrieveAttributes(&table_, &xml_parser_, attrs, arraysize(attrs),
values.data(), nullptr /*out_indices*/));
ASSERT_TRUE(RetrieveAttributes(&table_, &xml_parser_, attrs.data(), attrs.size(), values.data(),
nullptr /*out_indices*/));
uint32_t* values_cursor = values.data();
EXPECT_EQ(Res_value::TYPE_NULL, values_cursor[STYLE_TYPE]);
@@ -157,14 +156,13 @@ TEST_F(AttributeResolutionXmlTest, ThemeAndXmlParser) {
ResTable::Theme theme(table_);
ASSERT_EQ(NO_ERROR, theme.applyStyle(R::style::StyleTwo));
uint32_t attrs[] = {R::attr::attr_one, R::attr::attr_two, R::attr::attr_three,
R::attr::attr_four, R::attr::attr_five};
std::vector<uint32_t> values;
values.resize(arraysize(attrs) * 6);
std::array<uint32_t, 5> attrs{{R::attr::attr_one, R::attr::attr_two, R::attr::attr_three,
R::attr::attr_four, R::attr::attr_five}};
std::array<uint32_t, attrs.size() * STYLE_NUM_ENTRIES> values;
std::array<uint32_t, attrs.size()> indices;
ApplyStyle(&theme, &xml_parser_, 0 /*def_style_attr*/,
0 /*def_style_res*/, attrs, arraysize(attrs),
values.data(), nullptr /*out_indices*/);
ApplyStyle(&theme, &xml_parser_, 0 /*def_style_attr*/, 0 /*def_style_res*/, attrs.data(),
attrs.size(), values.data(), indices.data());
const uint32_t public_flag = ResTable_typeSpec::SPEC_PUBLIC;