Comments 39
> Больше всего меня порадовало предупреждение V401(можно уменьшить размер). Вот это я купил бы!
gcc -Wpadded расскажет вам о структурах и классах где вставлено выравнивание. Способ перестановки членов класса придётся придумать самостоятельно.
gcc -Wpadded расскажет вам о структурах и классах где вставлено выравнивание. Способ перестановки членов класса придётся придумать самостоятельно.
Хм… надо попробовать, а то есть в некоторых местах маленькие, но часто создающиеся и в большом количестве структуры.
Способ расстановки — в прядке убывания размера типа (если размер кратен степени двойки). Если размер не кратен (структуры, массивы), можно подмутить вручную или использовать что-то типа #pragma pack(0)
“V401. The structure's size can be decreased via changing the fields' order. <…>”Видимо, имеются в виду потери из-за выравнивания. В приведённом примере второе поле (адрес) должно быть выровнено по 4-байтовой границе. Поэтому после первого поля придётся добавить ещё три байта. А если переставить поля местами, то мелкие можно скомпоновать друг с другом, потери на выравнивание будут меньше.
На данный момент я не могу понять как это возможно
Мне кажется, что в 64-битах выравнивание идет по 8 байтным адресам, а так да… надо смотреть на типы данных, их размер и порядок выбирать так, чтобы они влезал группами по 8 (4 — для 32 бит) байт
Ну, я обобщённо говорил, естественно.
Ну да, в первой структуре каждое поле занимает по 8 байт: первое — из-за выравнивания второго, а последнее — для выравнивания того, что идёт за всей структурой. Во втором случае первое поле занимет 8 байт, за ним int — 4 байта и сразу за ним bool, оставшиеся 3 байта уходят на выравнивание следующих данных.
Вот тут можно посмотреть табличку: www.viva64.com/ru/b/0009/
В общем результаты напоминают вот эти, отсюда вывод напрашивается, что пока для Qt приложений инструмент вообще бесполезен и нужно их особенности отдельно учитывать и писать свои правила.
>>Больше всего меня порадовало предупреждение V401(можно уменьшить размер). Вот это я купил бы!
Тут палка о двух концах. Перегруппируете без паддинга, можете потерять скорость. Вообще компилятор должен сам оптимизировать поля структуры, с icc будет всеравно в каком порядке они у вас записаны.
Тут палка о двух концах. Перегруппируете без паддинга, можете потерять скорость. Вообще компилятор должен сам оптимизировать поля структуры, с icc будет всеравно в каком порядке они у вас записаны.
Это не так.
В перегрупированном варианте все поля все равно выровнены (т.е. не пересекают 8байтовые границы)
В перегрупированном варианте все поля все равно выровнены (т.е. не пересекают 8байтовые границы)
Не пересекают — да, но меньшее число полей расположено по кратным адресам, а значит теряется скорость
Еще раз, для тех кто в танке. Как вы поля не напишете компилятор перегруппирует их по своему.
Сообщение «The size can be reduced from N to K bytes,» опирается на неверную посылку, что компилятор кладет поля как ему написано.
Сообщение «The size can be reduced from N to K bytes,» опирается на неверную посылку, что компилятор кладет поля как ему написано.
Section 9.2.12:
Nonstatic data members of a (non-union) class declared without an intervening access-specifier are allocated so that later members have higher addresses within a class object. The order of allocation of nonstatic data members separated by an access-specifier is unspecified (11.1)"
т.е. перегрупировывать поля компилятору явно запрещено стандартом.
Nonstatic data members of a (non-union) class declared without an intervening access-specifier are allocated so that later members have higher addresses within a class object. The order of allocation of nonstatic data members separated by an access-specifier is unspecified (11.1)"
т.е. перегрупировывать поля компилятору явно запрещено стандартом.
Знание стандартов это конечно хорошо, но еще бы знать и реалии:
GCC:
-fpack-struct[=n]
Without a value specified, pack all structure members together without holes. When a value is specified (which must be a small power of two), pack structure members according to this value, representing the maximum alignment (that is, objects with default alignment requirements larger than this will be output potentially unaligned at the next fitting location.
ICC:
-fpack-struct Pack structure members together.
GCC:
-fpack-struct[=n]
Without a value specified, pack all structure members together without holes. When a value is specified (which must be a small power of two), pack structure members according to this value, representing the maximum alignment (that is, objects with default alignment requirements larger than this will be output potentially unaligned at the next fitting location.
ICC:
-fpack-struct Pack structure members together.
Эти ключи отключают padding, структуры просто будут записаны as is, невыровненными.
Я чет не пойму — к чему вы стремитесь.
Если вы хотите чтобы не было дырок — то это одно.
Если вы хотите чтобы всё было выровнено — это другое.
Если вы хотите чтобы не было дырок — то это одно.
Если вы хотите чтобы всё было выровнено — это другое.
struct LiseElement {
char *m_pNext;
int m_value;
bool m_isActive;
};
Нет дырок, все выровнено. Что не так?
Кроме того, вот то, что вы пишете:
char *m_pNext;
int m_value;
bool m_isActive;
};
Нет дырок, все выровнено. Что не так?
Кроме того, вот то, что вы пишете:
Как вы поля не напишете компилятор перегруппирует их по своему.
Сообщение «The size can be reduced from N to K bytes,» опирается на неверную посылку, что компилятор кладет поля как ему написано.
Это мягко, говоря, не так.
m_pNext всегда будет раньше, чем m_value.
Сообщение «The size can be reduced from N to K bytes,» опирается на неверную посылку, что компилятор кладет поля как ему написано.
Это мягко, говоря, не так.
m_pNext всегда будет раньше, чем m_value.
Добавьте еще однобайтовое поле и достигнете одновременно и выровненности и отсутствия дыр.
А то в этом примере и компилятор ас из положит их выровненно.
А то в этом примере и компилятор ас из положит их выровненно.
Как тогда столько лет уже работают бинарные протоколы, основанные на упакованных структурах?
Если компилятор начнет перемещать поля как попала, то как тогда работать, например, со структурой типа BITMAPFILEHEADER? Поля будут там, где их написал программист. Чтобы поменять их последовательность необходимо совершить специальные действия.
“V525. The code containing the collection of similar blocks. Check items X, Y, Z,… in lines N1, N2, N3,” – ложные срабатывания.А почему эти срабатывания ложные? Можно пару примеров кода?
По данному предупреждению в основном были обнаружены магические цифры в процедурах рисования, как то: установка различных отступов и смещений.Хм… Не знаю, что у вас за проект, но вам не кажется, что такие вещи как размер отступов желательно вынести в некий шаблон или, возможно, в описание стиля, подобное CSS? Во всяком случае отделить от кода. Иначе могут начаться проблемы при попытке поддержать различные скины или лэйауты.
>> А почему эти срабатывания ложные? Можно пару примеров кода?
Сорри.
>>А почему эти срабатывания ложные? Можно пару примеров кода?
>> Хм… Не знаю, что у вас за проект, но вам не кажется, что такие вещи как размер отступов желательно вынести в некий шаблон или, возможно, в описание стиля, подобное CSS? Во всяком случае отделить от кода. Иначе могут начаться проблемы при попытке поддержать различные скины или лэйауты.
Так и есть многое в CSS находится, но некоторые вещи в CSS сделать не удалось поэтому кое где эта проблема осталась. Плюс по этому предупреждению находятся и константы не отказываться же от констант совсем.
>>А почему эти срабатывания ложные? Можно пару примеров кода?
QString ContactComponentModel::formatPhone(QString& phone) {
if (phone.length() == 5) {
return phone.insert(2, '-').insert(4, '-');
// return phone.substring(0, 1) + "-" + phone.substring(1, 3) + "-" + phone.substring(3, 5);
}
if (phone.length() == 6) {
return phone.insert(2, '-').insert(5, '-');
// return phone.substring(0, 2) + "-" + phone.substring(2, 4) + "-" + phone.substring(4, 6);
}
if (phone.length() == 7) {
return phone.insert(3, '-').insert(6, '-');
// return phone.substring(0, 3) + "-" + phone.substring(3, 5) + "-" + phone.substring(5, 7);
}
if (phone.length() == 10) {
return phone.insert(4, '-').insert(7, '-').insert(9, '-');
// return phone.substring(0, 3) + "-" + phone.substring(3, 6) + "-" + phone.substring(6, 8) + "-" + phone.substring(8, 10);
}
if (phone.length() == 11) {
return phone.insert(1, '-').insert(5, '-').insert(9, '-').insert(12, '-');
// return phone.substring(0, 1) + "-" + phone.substring(1, 4) + "-" + phone.substring(4, 7) + "-" + phone.substring(7, 9) + "-" + phone.substring(9, 11);
}
return phone;
}
* This source code was highlighted with Source Code Highlighter.>> Хм… Не знаю, что у вас за проект, но вам не кажется, что такие вещи как размер отступов желательно вынести в некий шаблон или, возможно, в описание стиля, подобное CSS? Во всяком случае отделить от кода. Иначе могут начаться проблемы при попытке поддержать различные скины или лэйауты.
Так и есть многое в CSS находится, но некоторые вещи в CSS сделать не удалось поэтому кое где эта проблема осталась. Плюс по этому предупреждению находятся и константы не отказываться же от констант совсем.
а мне данный код глаза режет %)
сделал бы как-нить вроде этого:
QString ContactComponentModel::formatPhone(QString& phone) {
// числа по основанию 8
const char *fix[] = {
"\05\02\04",
"\06\02\05",
"\07\03\06",
"\12\04\07\11",
"\13\01\05\11\14"
};
for (int i = 0, size = sizeof(fix)/sizeof(char*); i < size; i++) {
const char *s = fix[i];
if (phone.length() != *s)
continue;
while (*(++s))
phone.insert(*s, QLatin1Char('-'));
}
return phone;
}
и кода меньше, и копипасты меньше)
P.S. substring с последующим склеиванием работает гораздо медленнее, чем создание копии строки и последующее выполнение в нее insert'ов :)
сделал бы как-нить вроде этого:
QString ContactComponentModel::formatPhone(QString& phone) {
// числа по основанию 8
const char *fix[] = {
"\05\02\04",
"\06\02\05",
"\07\03\06",
"\12\04\07\11",
"\13\01\05\11\14"
};
for (int i = 0, size = sizeof(fix)/sizeof(char*); i < size; i++) {
const char *s = fix[i];
if (phone.length() != *s)
continue;
while (*(++s))
phone.insert(*s, QLatin1Char('-'));
}
return phone;
}
и кода меньше, и копипасты меньше)
P.S. substring с последующим склеиванием работает гораздо медленнее, чем создание копии строки и последующее выполнение в нее insert'ов :)
сорри, форматирование забыл
QString ContactComponentModel::formatPhone(QString& phone) {
const char *fix[] = {
"\05\02\04",
"\06\02\05",
"\07\03\06",
"\12\04\07\11",
"\13\01\05\11\14"
};
for (int i = 0, size = sizeof(fix)/sizeof(char*); i < size; i++) {
const char *s = fix[i];
if (phone.length() != *s)
continue;
while (*(++s))
phone.insert(*s, QLatin1Char('-'));
}
return phone;
}
* This source code was highlighted with Source Code Highlighter.Думаю Вы правы, вообще в коде достаточно не оптимальности потому что сроки как всегда поджимают. Поэтому я и говорю что если бы инструмент предлагал оптимизации было бы здорово.
Вроде: «substring с последующим склеиванием работает гораздо медленнее, чем создание копии строки и последующее выполнение в нее insert'ов»
Впрочем я же как раз использую insert а не substring, т.е. Ваш вариант будет быстрее в плане производительности, как Вы считаете?
Вроде: «substring с последующим склеиванием работает гораздо медленнее, чем создание копии строки и последующее выполнение в нее insert'ов»
Впрочем я же как раз использую insert а не substring, т.е. Ваш вариант будет быстрее в плане производительности, как Вы считаете?
Мне из прежних описания PVS-Studio казалось, что предупреждения от Вива64 можно выключить, и они не будут «путаться» под ногами. Убрали из бесплатной версии?
Воообще, если нельзя конкретные предупреждения убрать по типам, неудобно, да…
Возможность отключать конкретные файлы по маске тоже была бы полезна, факт.
Про магик нумберс не соглашусь — их таки стоит убрать хотя бы под дефайны, а лучше в константы. Сами же будете рады, что это сделано, когда придется искать по коду, например…
Кстати, из вашего описания у меня появилась идея для авторов: а не сделать ли им возможность покупать диагностики «поштучно»? :) Будет лишний повод попробовать и триальную версию, и первую покупку чтобы «втянуться» будет легче сделать…
Воообще, если нельзя конкретные предупреждения убрать по типам, неудобно, да…
Возможность отключать конкретные файлы по маске тоже была бы полезна, факт.
Про магик нумберс не соглашусь — их таки стоит убрать хотя бы под дефайны, а лучше в константы. Сами же будете рады, что это сделано, когда придется искать по коду, например…
Кстати, из вашего описания у меня появилась идея для авторов: а не сделать ли им возможность покупать диагностики «поштучно»? :) Будет лишний повод попробовать и триальную версию, и первую покупку чтобы «втянуться» будет легче сделать…
Надо было намеренные ошибки внести — результаты могли оказаться интереснее :)
Большое спасибо за статью! Я очень рад ее появлению. И не важно, положительная она или нет. Главное, что появился интерес к нашему инструменту. А теперь приступлю делать мир лучше:
1) Прошу сгенерировать для одного файла, где было выдано сообщение V001, препроцессированный i-файл. Быть может, после правки отчет будет смотреться более весело. :)
2) Я хочу отправить Вам регистрационный ключ для некоммерческого использования. Прошу написать мне в почту — karpov[@]viva64.com. Заодно, быть может, что-то новое найдется. У нас в каждой бете появляются новые диагностические правила.
3) По поводу цитат из документации. Хочу просто напомнить, что на сайте вся документация доступна также на русском языке. Возможно, кому то так будет проще.
4) Отвечу на вопрос, почему триальны сообщения V126. Ведь и просто поиском можно искать long. Просто там общий механизм для всех правил. Выдается случайным образом позиции для 25% сообщений: unsigned(rand() % 100) < 25.
5) Почему V112 размазано по 2 и 3 уровню? На третий уровень отправляются такие числа 4, 32. Уж очень много ложных срабатываний с ними. На втором уровне числа: 0x7fffffff, 0x80000000, 0xffffffff. Быть может все в третий уровень перенести?
6) На тему уменьшения размера структур: "Урок 23. Паттерн 15. Рост размеров структур".
7) Чтобы не смотреть ошибки, относящиеся к исходным файлам QT, необходимо добавить папку c QT в исключения. В настройке «Don't Check Files» укажите «X:\MyWork\MyQT\».
1) Прошу сгенерировать для одного файла, где было выдано сообщение V001, препроцессированный i-файл. Быть может, после правки отчет будет смотреться более весело. :)
2) Я хочу отправить Вам регистрационный ключ для некоммерческого использования. Прошу написать мне в почту — karpov[@]viva64.com. Заодно, быть может, что-то новое найдется. У нас в каждой бете появляются новые диагностические правила.
3) По поводу цитат из документации. Хочу просто напомнить, что на сайте вся документация доступна также на русском языке. Возможно, кому то так будет проще.
4) Отвечу на вопрос, почему триальны сообщения V126. Ведь и просто поиском можно искать long. Просто там общий механизм для всех правил. Выдается случайным образом позиции для 25% сообщений: unsigned(rand() % 100) < 25.
5) Почему V112 размазано по 2 и 3 уровню? На третий уровень отправляются такие числа 4, 32. Уж очень много ложных срабатываний с ними. На втором уровне числа: 0x7fffffff, 0x80000000, 0xffffffff. Быть может все в третий уровень перенести?
6) На тему уменьшения размера структур: "Урок 23. Паттерн 15. Рост размеров структур".
7) Чтобы не смотреть ошибки, относящиеся к исходным файлам QT, необходимо добавить папку c QT в исключения. В настройке «Don't Check Files» укажите «X:\MyWork\MyQT\».
Написал Вам письмо по поводу 1 и 2 пункта.
Документацию на русском нашел только после опубликования статьи, было бы здорво если PVS сам определял какой язык по умолчанию в системе. По умолчанию у меня русский язык, но документация была на английском.
Урок 23, обязательно прочту спасибо. По поводу 7 пункта — допускаю что смотрел не внимательно и не увидел, дома приду гляну еще раз.
Документацию на русском нашел только после опубликования статьи, было бы здорво если PVS сам определял какой язык по умолчанию в системе. По умолчанию у меня русский язык, но документация была на английском.
Урок 23, обязательно прочту спасибо. По поводу 7 пункта — допускаю что смотрел не внимательно и не увидел, дома приду гляну еще раз.
>> Многие предупреждения ссылаются на автогенерируемые файлы, вроде moc*.cpp.
>> Было бы удобнее для меня, если бы PVS не пытался анализировать эти файлы.
В настройках можно вот прямо такую маску задать и они не будут проверяться. Все уже есть.
>> Было бы удобнее для меня, если бы PVS не пытался анализировать эти файлы.
В настройках можно вот прямо такую маску задать и они не будут проверяться. Все уже есть.
Sign up to leave a comment.
Опыт статического анализа Qt-программы с использованием PVS-Studio