[prog.bugs] А жуки все лезут и лезут. На этот раз приплюснутые

Прошло совсем немного спокойных дней после очень нервной bug’s hunting week, как опять полезли злобные жуки, в самых неожиданных местах (в коде, который эксплуатировался годами) и в самый неподходящий момент. Причем оба бага, о которых я расскажу, являются просто таки классическими C/C++ багами.

Первый баг очень простой:

std::string hex_char( char ch ) {    char v[5];    std::sprintf( v, "%02x", ch );    return std::string(v); }

Прожил во вспомогательной утилитке несколько лет. Которой не то, чтобы пользовались каждый день, но время от времени запускали.

Если кто-то не силен в С++ или не заметил еще проблемы, раскрою причину. Тип char в большинстве случаев (если не указывать компилятору специальный ключик) знаковый. При передаче ch в sprintf происходит его преобразование в int. Т.е., если в ch находился символ из диапазона 128..255 (например, 0xFF), то при передаче в sprintf получится значение 0xFFFFFFFF (для 32-х битового int-а). Соответственно, текстовое представление этого значения никак не поместится в v. И, в лучшем случае, программа сразу упадет. Что она и начала делать, когда я с ее помощью стал проверять изменения, внесенные в совсем-совсем другой части большого программного комплекса.

Второй баг так же не сложен. Но вот его маскировка в течении трех лет намного интереснее. Впрочем, начнем по порядку. Ошибка находится в следующем фрагменте:

std::string msg; std::vector< wchar_t > wbuf; ... msg = ACE_Wide_To_Ascii::convert( &( wbuf.front() ) );

В чем здесь смысл – из какого-то источника берется последовательность байт, которая является набором Unicode-символов. Эта последовательность сохраняется в wbuf. А затем Unicode-строка передается в статический метод convert ACE-овского класса ACE_Wide_To_Ascii. Данный метод имеет следующий прототип:

/// Converts an wchar_t string to ascii and returns a new string. static char *convert (const wchar_t *wstr);

Сразу расскажу, что здесь к чему. Во-первых, convert возвращает указатель на динамически размещенную в памяти строку. Эту память затем нужно освобождать вручную. Чего не делалось – получалась утечка. Во-вторых, convert не добавляет к результирующей строке завершающий 0-символ. Поэтому результат convert-а нельзя отдавать в basic_string::operator=(), т.к. operator=() определяет конец строки как раз по 0-символу. Но 0-символа не было, поэтому получался проход по памяти.

Для меня очевидно, что эти две проблемы не возникли бы вообще, если бы разработчик заглянул внутрь ACE_Wide_To_Ascii::convert. Мне вот из комментария к методу было не понятно, как метод работает и что возвращает. Пришлось залезть в исходник и разобраться. Почему этого не сделал автор кода – теперь уже не узнать :(

Но самое интересное не в факте существования бага. Интересно другое – долгое время этот код успешно проходил unit-тестирование. Т.е. он изначально был покрыт unit-тестом и все время проверялся. Но ошибка не проявлялась.

Оказалось, что вначале unit-тесты писались с использованием Boost.Test. При этом unit-тесты отрабатывали тихо и гладко, никаких проблем не диагностировалось. Затем Boost.Test выбросили, unit-тесты чуть подкорректировали – и… И ошибка не проявлялась еще в течении нескольких лет.

И вот что подозрительно. Как только я запустил полный билд библиотеки с прогоном всех тестов, проблема с проходом по памяти сразу же всплыла наружу. Почему же она не была выявлена ранее? Либо у разработчика при прогоне тестов звезды располагались совсем иначе и проход по памяти по счастливому стечению обстоятельств не происходил. Либо разработчик вообще не запускал unit-тесты – т.е. увидел, что компиляция проходит успешно, а запускать не стал. К сожалению, правды сейчас не узнать.

В очередной раз убеждаюсь, что C++ – это классный язык, если разработку ведет совсем маленькая команда хороших специалистов. Но стоит к разработке присоединиться парочке чайников или, что еще хуже, раздолбаев, как C++ный проект превращается в минное поле.