Комментарии 15
Можно было бы попробовать собрать и загрузить Tizen для статического анализа coverity и посмотреть, что выдаст этот анализатор.
- Статический анализ Tizen — coverity
- Статический анализ Tizen — cppcheck
- Статический анализ Tizen — Clang Static Analyzer
if (__builtin_expect(smth == NULL, 0))
return EINVAL;
if (__builtin_expect(user_id != owner_id, 0))
return EACCES;
return 0;
Приведенный выше код GCC скомпилирует как:
if (smth != NULL)
{
if (user_id == owner_id)
{
return 0;
}
return EACCES;
}
return EINVAL;
В приведенных вами фрагментах кода очень много таких мест, где условия идут в «неверном» порядке.
Осторожнее с диагностикой V813 — замена значения на константную ссылку может поломать функцию если ее реализация где-то полагается на тот факт, что разные переменные указывают на разные участки памяти.
Например:
В результате портирования появилась реализация в которой было убрано явное копирование данных и написали так:
void foo(type o)
{
...
Вместо того чтобы написать так:
void foo(const type& o)
{
string copy_of_o = o;
...
А далее в коде используется сравнение адресов на члены класса, ну или возможно используется this как id объекта, что в целом не очень хорошая затея и часто указывает на плохой дизайн.
P.S. всё же прошу пример ибо подозреваю, что неправильно понял вами написанное, но ошибка кажется интересной :)
Ну вот например код с неоптимальной передачей параметров:
const uint32_t BASE = 1000000000;
void mul_add(std::vector<uint32_t> a, std::vector<uint32_t> b, std::vector<uint32_t>& c) {
if (c.size() < a.size() + b.size() - 1)
c.resize(a.size() + b.size() - 1);
bool carry_on_last_iter = false;
for (size_t i = 0; i < a.size(); i++) {
uint64_t s = 0, ai = a[i];
for (size_t j = 0; j < b.size(); j++) {
s += c[i + j] + ai * b[j];
c[i + j] = s % BASE;
s /= BASE;
}
carry_on_last_iter = false;
if (s > 0) {
if (c.size() <= i + b.size())
c.push_back(s);
else {
c[i + b.size()] += s;
carry_on_last_iter = true;
}
}
}
if (carry_on_last_iter) {
for (size_t i = a.size() + b.size() - 1; c[i] > BASE; i++) {
c[i] -= BASE;
if (c.size() <= i)
c.push_back(1);
else
c[i + 1]++;
}
}
}
А что случится если вызвать mul_add(a, a, a)
, а потом попытаться избежать копирования объявив параметры a и b как const&
?
А что случится если вызвать mul_add(a, a, a), а потом попытаться избежать копирования объявив параметры a и b как const&?
Ничего хорошего. Но опять же, тут вопрос архитектуры этого API и в общем то разработчик ССЗБ. Накладные расходы на копирование векторов всё же не маленькие. Кроме того объекты a и b по логике работы функции всё равно константы, а в параметрах const забыт. В общем не могу я назвать приведённый код хорошим. Если код часто используемый (горячий в профилировщике) то в функцию следовало бы добавить отладочные проверки:
void mul_add(const std::vector<uint32_t>& a, const std::vector<uint32_t>& b, std::vector<uint32_t>& c) {
assert(a.this != c.this); // use mul_add_same
assert(b.this != c.this);
if (c.size() < a.size() + b.size() - 1)
c.resize(a.size() + b.size() - 1);
и добавить обвёртки mul_add_same для возможности работы с одним и тем же вектором, а лучше вообще дописать реализации для работы с одним и тем же вектором, да это дольше по времени, зато код будет значительно быстрее.
Если же код не часто используемый или вообще крайне редко используемый (не горячий в профилировщике), то необходимо явно дать понять и написать что функция обязана работать с копией данных. Т.е. вот так:
void mul_add(const std::vector<uint32_t>& _a, const std::vector<uint32_t>& _b, std::vector<uint32_t>& c) {
const auto a(_a), b(_b);
if (c.size() < a.size() + b.size() - 1)
c.resize(a.size() + b.size() - 1);
… делать так необходимо чтобы код в первую очередь был явно читаем и интерпретируем человеком.
Отладочные проверки это хорошо (я сам такие же в реальном коде ставил) — но я не случайно написал вызов mul_add(a, a, a)
первым. В данном случае, предлагаемая незначительная оптимизация ломает уже написанный код.
Почему оптимизация незначительная? Да потому что копирование векторов обычно работает намного быстрее чем два вложенных цикла в теле функции!
по результатам диагностики V803 предлагаю переименовать язык C++ в ++C.
Поговорим о микрооптимизациях на примере кода Tizen