Вебинар: C# разработка и статический анализ: в чем практическая польза? - 18.11
Tesseract - свободная компьютерная программа для распознавания текстов, разрабатываемая компанией Google. В описании проекта говорится: "Tesseract is probably the most accurate open source OCR engine available". А давайте попробуем, сможет ли статический анализатор PVS-Studio распознать какие-то ошибки в этом проекте.
Tesseract - свободная компьютерная программа для распознавания текстов, разрабатывавшаяся Hewlett-Packard с середины 1980-х по середину 1990-х, а затем 10 лет "пролежавшая на полке". В августе 2006 г. Google купил её и открыл исходные тексты под лицензией Apache 2.0 для продолжения разработки. В настоящий момент программа уже работает с UTF-8, поддержка языков (включая русский) осуществляется с помощью дополнительных модулей. [описание взято из Wikipedia]
Исходный код проекта доступен на сайте Google Code: https://code.google.com/p/tesseract-ocr/
Объем исходного кода около 16 мегабайт.
Приведу фрагменты кода, на которые я обратил внимание, просматривая отчёт PVS-Studio. Возможно я что-то упустил. Поэтому создателям Tesseract целесообразно провести проверку самостоятельно. Пробная версия, активна 7 дней, что более чем достаточно для такого небольшого проекта. Ну а потом им решать, хотят они регулярно использовать инструмент и находить опечатки, или нет.
Как всегда, напомню. Суть статического анализа не в разовых проверках, а в регулярном использовании.
void LanguageModel::FillConsistencyInfo(....)
{
....
float gap_ratio = expected_gap / actual_gap;
if (gap_ratio < 1/2 || gap_ratio > 2) {
consistency_info->num_inconsistent_spaces++;
....
}
Предупреждения PVS-Studio: V636 The '1 / 2' expression was implicitly casted from 'int' type to 'float' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. language_model.cpp 1163
Переменную 'gap_ratio' хотят сравнить со значением 0.5. К сожалению, выбран неудачный способ записать 0.5. Деление 1/2 является целочисленным и даёт в результате 0.
Корректный код должен быть таким:
if (gap_ratio < 1.0f/2 || gap_ratio > 2) {
Или таким:
if (gap_ratio < 0.5f || gap_ratio > 2) {
Есть и другие места, где осуществляется подозрительное целочисленное деление. Возможно, среди них есть действительно неприятные ошибки.
Фрагменты кода, которые стоит проверить:
uintmax_t streamtoumax(FILE* s, int base) {
int d, c = 0;
....
c = fgetc(s);
if (c == 'x' && c == 'X') c = fgetc(s);
....
}
Предупреждение PVS-Studio: V547 Expression 'c == 'x' && c == 'X'' is always false. Probably the '||' operator should be used here. scanutils.cpp 135
Правильный вариант проверки:
if (c == 'x' || c == 'X') c = fgetc(s);
Обнаружилась одна интересная конструкция. Я такого ещё не видел:
void TabVector::Evaluate(....) {
....
int num_deleted_boxes = 0;
....
++num_deleted_boxes = true;
....
}
Предупреждение PVS-Studio: V567 Undefined behavior. The 'num_deleted_boxes' variable is modified while being used twice between sequence points. tabvector.cpp 735
Не понятно, что хотел сказать этим кодом автор. Скорее всего этот код является последствием опечатки.
Результат выражения предсказать невозможно. Переменная 'num_deleted_boxes' может быть увеличена как до присваивания, так и после. Причина - переменная меняется дважды в одной точке следования.
Остальные ошибки, приводящие к неопределенному поведению, связаны с использованием сдвигов. Рассмотрим пример:
void Dawg::init(....)
{
....
letter_mask_ = ~(~0 << flag_start_bit_);
....
}
Предупреждение V610 Undefined behavior. Check the shift operator '<<. The left operand '~0' is negative. dawg.cpp 187
Выражение '~0' имеет тип 'int' и равно значению '-1'. Сдвиг отрицательных значений приводит к неопределённому поведении. То, что программа может корректно работать, является везением и не больше того. Можно исправить недочёт, сделав '0' беззнаковым:
letter_mask_ = ~(~0u << flag_start_bit_);
Однако это ещё не всё. Анализатор выдаёт на эту строку ещё одно предупреждение:
V629 Consider inspecting the '~0 << flag_start_bit_' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. dawg.cpp 187
Дело в том, что переменная 'letter_mask_' имеет тип 'uinT64'. Как я понимаю, может быть необходимо записывать единицы в старшие 32 бита. В таком случае, созданное выражение некорректно. Оно работает только с младшими битами.
Нужно сделать так, чтобы '0' был 64-битным типом. Исправленный вариант:
letter_mask_ = ~(~0ull << flag_start_bit_);
Перечислю списком другие фрагменты кода, где осуществляется сдвиг отрицательных чисел:
TESSLINE* ApproximateOutline(....) {
EDGEPT *edgept;
....
edgept = edgesteps_to_edgepts(c_outline, edgepts);
fix2(edgepts, area);
edgept = poly2 (edgepts, area); // 2nd approximation.
....
}
Предупреждение PVS-Studio: V519 The 'edgept' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 76, 78. polyaprx.cpp 78
Ещё один похожий случай:
inT32 row_words2(....)
{
....
this_valid = blob_box.width () >= min_width;
this_valid = TRUE;
....
}
Предупреждение PVS-Studio: V519 The 'this_valid' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 396, 397. wordseg.cpp 397
В начале рассмотрим класс 'MasterTrainer'. Обратите внимание, что член класса 'samples_' расположен до члена 'fontinfo_table_':
class MasterTrainer {
....
TrainingSampleSet samples_;
....
FontInfoTable fontinfo_table_;
....
};
Согласно стандарту порядок инициализация членов класса в конструкторе происходит в порядке их объявления в классе. Это значит, что 'samples_' будет инициализироваться ДО инициализации 'fontinfo_table_'.
Теперь рассмотрим конструктор:
MasterTrainer::MasterTrainer(NormalizationMode norm_mode,
bool shape_analysis,
bool replicate_samples,
int debug_level)
: norm_mode_(norm_mode), samples_(fontinfo_table_),
junk_samples_(fontinfo_table_),
verify_samples_(fontinfo_table_),
charsetsize_(0),
enable_shape_anaylsis_(shape_analysis),
enable_replication_(replicate_samples),
fragments_(NULL), prev_unichar_id_(-1),
debug_level_(debug_level)
{
}
Беда в том, что для инициализации 'samples_' используется ещё неинициализированная переменная 'fontinfo_table_'.
Аналогичная ситуация в этом классе с инициализацией полей 'junk_samples_' и 'verify_samples_'.
Я не берусь сказать, как лучше поступить с этим классом. Возможно будет достаточно перенести объявление 'fontinfo_table_' в самое начало класса.
Опечатку заметить не просто, но анализатор не знает усталости.
class ScriptDetector {
....
int korean_id_;
int japanese_id_;
int katakana_id_;
int hiragana_id_;
int han_id_;
int hangul_id_;
int latin_id_;
int fraktur_id_;
....
};
void ScriptDetector::detect_blob(BLOB_CHOICE_LIST* scores) {
....
if (prev_id == katakana_id_)
osr_->scripts_na[i][japanese_id_] += 1.0;
if (prev_id == hiragana_id_)
osr_->scripts_na[i][japanese_id_] += 1.0;
if (prev_id == hangul_id_)
osr_->scripts_na[i][korean_id_] += 1.0;
if (prev_id == han_id_)
osr_->scripts_na[i][korean_id_] += kHanRatioInKorean;
if (prev_id == han_id_) <<<<====
osr_->scripts_na[i][japanese_id_] += kHanRatioInJapanese;
....
}
Предупреждение PVS-Studio: V581 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 551, 553. osdetect.cpp 553
Скорее всего, самое последние сравнение должно быть таким:
if (prev_id == japanese_id_)
Не нужно проверять, что возвращает оператор 'new'. Если не удастся выделить память, возникнет исключение. Конечно, можно сделать специальный оператор 'new', который возвращает нулевые указатели, но это отдельный случай (подробности).
В результате, вот такую функцию можно упростить:
void SetLabel(char_32 label) {
if (label32_ != NULL) {
delete []label32_;
}
label32_ = new char_32[2];
if (label32_ != NULL) {
label32_[0] = label;
label32_[1] = 0;
}
}
Предупреждение PVS-Studio: V668 There is no sense in testing the 'label32_' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. char_samp.h 73
Есть ещё 101 место, где проверяется указатель, который вернул оператор 'new'. Перечислять их в статье я не вижу смысла. Проще запустить для этого PVS-Studio.
Используйте статический анализ регулярно, и вы сэкономите массу времени на решение более полезных задач, чем отлов глупых ошибок и опечаток.
И не забывайте следовать за мною в Twitter: @Code_Analysis. Я регулярно публикую ссылки на интересные статьи по тематике Си++.
0